Fix crash of :debug-log-filter when --filter wasn't given #2505
Conversation
|
|
||
| Scenario: Using debug-log-filter | ||
| When I run :debug-log-filter webview | ||
| Then no crash should happen |
There was a problem hiding this comment.
We run all tests in one .feature file in the same qutebrowser instance for performance reasons, and most tests work by parsing qutebrowser's log - so this will probably break all tests following this one.
Even if we had a way to unset a filter with :debug-log-filter, the test would probably fail trying to execute that... so I don't really know a good solution for this. Do you?
There was a problem hiding this comment.
Oh, I did't realize that. I need to think about a way to do this (this is one of those cases where you need tests for your tests >_<).
There was a problem hiding this comment.
Worst-case scenario btw we just don't add the test ... Although that's obviously not ideal.
There was a problem hiding this comment.
Right - I'm fine with that for the record FWIW if nothing else works.
What could work is to immediately reset the filter after setting it (and maybe testing if it works, with :pyeval or so maybe?), and initially setting it to something which still outputs enough for the test infrastructure to work (something like ipc,command or so)
There was a problem hiding this comment.
I moved it out of the BDD tests in to a regular pytest test. Seems to work well.
To reset the filter I had to add a special none argument.
There was a problem hiding this comment.
So it looks like test_*.py files need to be unique globally across the entire project?
I worked around this by choosing a silly filename to test this hypothesis, but this seems like a strange requirement?
Any advice? I can dig deeper later if you don't know.
Codecov Report
@@ Coverage Diff @@
## master #2505 +/- ##
==========================================
+ Coverage 81.07% 81.07% +<.01%
==========================================
Files 132 132
Lines 18271 18277 +6
Branches 2764 2766 +2
==========================================
+ Hits 14813 14818 +5
- Misses 2971 2974 +3
+ Partials 487 485 -2
Continue to review full report at Codecov.
|
There was no `LogFilter`. The fix is to always initialize a `LogFilter()` with `None`. as the "filter". Fixes #2303.
|
As discussed in IRC, you can simply use this as test, in Scenario: Using debug-log-filter
When I run :debug-log-filter commands,ipc,webview
And I run :enter-mode insert
And I run :debug-log-filter none
And I run :leave-mode
Then "Entering mode KeyMode.insert *" should not be loggedAs for requiring unique filenames, this section in the pytest docs explains why. |
This allows us to clear any filters. Useful for users, and needed for the tests.
|
Okay, should be good to go now. I'm not sure why codecov/patch fails and thinks only 70% of this diff is tested, since that's not true as far as I can figure out (?!) |
|
See https://codecov.io/gh/qutebrowser/qutebrowser/pull/2505/diff - seems fine to me though. |
|
Thanks! |
There was no
LogFilter. The fix is to always initialize aLogFilter()withNone. as the "filter".Fixes #2303.
This change is