-
Notifications
You must be signed in to change notification settings - Fork 330
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix logging demo test #400
Conversation
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
At first it wasn't obvious to me either. But deep down, it's because of this. That patch essentially duplicates test case classes through inheritance to prevent overlap of arguments bound to class methods for launch tests that use parameterization -- there's only one class definition and N different set of arguments to be bound. Now, because of that, when I'm not particularly happy with either change, but I deem this one less harmful. The true problem is that |
Or, alternatively, we can forbid |
@@ -38,12 +38,9 @@ def generate_test_description(ready_fn): | |||
|
|||
|
|||
class TestLoggingDemo(unittest.TestCase): | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might want to keep this empty line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 9056e2d
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Merging |
By relocating
output_filter
assignment. Running CI for Linux only as changes are cross-platform.