-
Notifications
You must be signed in to change notification settings - Fork 0
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
[BEAM-5123] Testsuite do not run all the tests it should #18
Conversation
for (Class<?> testClass : testClasses) { | ||
LOG.info(String.format("Testkit will schedule tests in '%s'.", testClass)); |
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.
Meanwhile exception messages doesnt provide good solution for formatting, logging methods do. I would prefer, String.format is uselesss here or is there some benefit?
LOG.info("Testkit will schedule tests in {}.", testClass);
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.
No benefit. Only the info
method has unclear contract to me. I do not know how to log Throwable
with custom message without putting Throwable
as last formatting parameter. Something which can potentially break once SLF4J implementation changes.
@@ -59,7 +59,9 @@ public TestSuiteRunner(Class<?> klass) throws Throwable { | |||
BeamRunnerWrapper runner = |
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.
Its not used now, why is it there?
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.
Removed. Thanks.
Few minor changes and we are good to go :) |
ea13274
to
ee0b7d7
Compare
…OperatorTestSuite` fixed by moving it. It forces explicit imports. Some new info level logs added.
…d since it is both not working with `BeamFlow` and unnecessary due to translation to Beam. Small documentation and test changes.
…such as `ClassAwareKryoCoder` removed.
56a4d6f
to
5f552a4
Compare
Class import collision unearthed and fixed. New logs recording test suite test methods resolving added. Small cleanup (unused parameter, unwanted TODO).
Follow this checklist to help us incorporate your contribution quickly and easily:
[BEAM-XXX] Fixes bug in ApproximateQuantiles
, where you replaceBEAM-XXX
with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.It will help us expedite review of your Pull Request if you tag someone (e.g.
@username
) to look at it.Post-Commit Tests Status (on master branch)