Introduce Dropwizard Metrics integration test #1426
Conversation
|
Looks good. No mutations were possible for these changes. |
|
Looks good. No mutations were possible for these changes. |
Stephan202
left a comment
Stephan202
left a comment
There was a problem hiding this comment.
Pushed more changes; getting closer.
| + <arg>-Xmaxwarns</arg> | ||
| + <arg>1000000</arg> |
There was a problem hiding this comment.
I added this for consistency with the Checkstyle setup. The fact that right now it doesn't have impact is a function of our checks and the code under test.
(The alternative is to drop -Xmaxwarns from both integration tests. This reduces patch maintenance at the cost of a potentially incomplete list of warnings. I guess it's fine to consistently apply the flag, for now.)
integration-tests/metrics.sh
Outdated
| "${patch_error_prone_flags}" \ | ||
| "${validation_error_prone_flags}" \ | ||
| "${validation_build_flags}" \ | ||
| ${@} |
There was a problem hiding this comment.
On Mac this gives an:
$ ./integration-tests/metrics.sh
./integration-tests/metrics.sh: line 26: @: unbound variableIf I drop the {} it works 🤔.
There was a problem hiding this comment.
Then let's drop it. (Also for Checkstyle.) Weird stuff :)
|
I pushed a few things that also impact the expected warnings, with which JDK version did you build exactly? I feel I'm introducing more now 🤔. |
|
Looks good. No mutations were possible for these changes. |
Stephan202
left a comment
Stephan202
left a comment
There was a problem hiding this comment.
I pushed a few things that also impact the expected warnings, with which JDK version did you build exactly? I feel I'm introducing more now 🤔.
Not just more, also a few less. Very weird; can't explain yet. (But the new behavior is consistent; also observe this locally and on GitHub Actions.)
Added two more commits. @mohamedsamehsalah this PR is ready for your review 😄
Only open point is to revert some .github/workflows/run-integration-tests.yml changes. Will do if the tests pass.
integration-tests/metrics-init.patch
Outdated
| @@ -350,7 +383,7 @@ | ||
| <artifactId>maven-javadoc-plugin</artifactId> | ||
| <version>3.10.1</version> | ||
| <configuration> | ||
| - <source>8</source> | ||
| + <source>11</source> | ||
| <doclint>none</doclint> | ||
| <quiet>true</quiet> | ||
| <notimestamp>true</notimestamp> |
There was a problem hiding this comment.
We can omit this, as we don't run Javadoc during the integration test.
|
Looks good. No mutations were possible for these changes. |
|
Looks good. No mutations were possible for these changes. |
|
Thanks @mohamedsamehsalah !
Did this 😄. Will run the tests once more to see if everything is good to go! |
|
/integration-test |
|
Looks good. No mutations were possible for these changes. |
|
Looks good. No mutations were possible for these changes. |
|
Stephan202
left a comment
Stephan202
left a comment
There was a problem hiding this comment.
Added one more commit. Suggested commit message:
Introduce Dropwizard Metrics integration test (#1426)
Stephan202
changed the title
Credits to @oxkitsune for the initial work on this integration test!
A continuation of: #894.