Show a "Loading ..." view instead of a "No storage-time" view when starting up #424
Conversation
This helps somewhat with readability of both UsageView.__init__ and of the individual pieces of layout code themselves.
|
Clearly CI is unhappy with this. I'm working on further changes to fix that. |
Then use this to give that widget a writeable configuration file when the tests are running.
R0201: Method could be a function (no-self-use)
|
Seems like all of the pieces now line up just right now, so maybe it's reasonable to have a human review at this point... |
crwood
left a comment
There was a problem hiding this comment.
Thanks a lot for this.
It took me a bit to grok this (due to the addition of attrs -- which I wasn't terribly familiar with) but this looks good to me and takes an approach that I would more or less as I would expect (i.e., of providing a different default view and displaying the chart only when token amounts can be discovered). We can indeed improve the wording/presentation of the "loading" label later on; the important thing, for now, I think, is to guard against giving users the false impression that they have no storage-time remaining.
Anyway, merging... And thanks again!
| debug_exporter: DebugExporter = attr.ib() | ||
|
|
||
| @welcome_dialog.default | ||
| def _default_welcome_dialog(self): |
There was a problem hiding this comment.
A very small nitpick (since the scope of these wrapped methods is also very small) but consider using a consistent naming scheme here; some of these methods follow the naming pattern of _x_default while others follow _default_x (I have no preference as to which of the two forms is better -- and won't hold up merging this because of something so trivial -- I just wanted to point it out).
There was a problem hiding this comment.
Huh. Yes, I agree consistent naming would be good here. I was trying to be consistent, in fact, I guess I should have tried harder!
|
|
||
| # Without this, QWidget() aborts the process with "QWidget: Must construct a | ||
| # QApplication before a QWidget". | ||
| app = QApplication([]) |
There was a problem hiding this comment.
FWIW, this is only necessary if you're running the gui tests in isolation (e.g., via pytest tests/gui or similar). If the test suite is run in its entirety (e.g., by running just pytest or pytest tests in the top level of the source tree), the pytest-qt plugin will initialize and use it's own QApplication instance throughout the run. I'm not sure why it doesn't work the same way in both instances, however... Anyway, there's no harm in keeping this one here; (Py)Qt will just use whichever QApplication instance is available first.
Fixes #423
This is my first attempt at writing pytest tests for Qt application code. I took a guess at what a nice shape might be. I'm not sure why I had to add the
QApplication([])call. It looks like there are already tests for Qt application code... but they fail (as do my new ones) in my environment without this.I didn't put a lot of thought into the actual words to show here, I mostly spent my effort figuring out how to get things to show up at the right time at all, so more than happy to have edits suggested for the text.
Also ... I did quite a bit of refactoring before figuring out what was going on here. I can split that into a separate PR if that would help.
Basically 2bfc26d is the fix for the behavior, 951f343 are the unit tests, and the rest are refactoring that should change no behavior.