convert instrument jdbc test from groovy to java#12132
Conversation
Co-authored-by: Jay DeLuca <jaydeluca4@gmail.com>
Co-authored-by: Jay DeLuca <jaydeluca4@gmail.com>
|
It is look like failed by Address already in use. It is not break by jdbc test. the jdc case is all memory server |
| @DisplayName( | ||
| "basic statement with #connection.getClass().getCanonicalName() on #system generates spans") |
There was a problem hiding this comment.
I'm not sure about the value of this. Spock is able to replace #connection.getClass().getCanonicalName() and #system with actual parameter values so you'd have a test named prepared statement execute on h2 with org.h2.jdbc.JdbcConnection generates a span with junit this does not happen
There was a problem hiding this comment.
junit can not do the same ability,like getCannicalname。
Junit is a tree struct, DisplayName for the method, parametertest have each name。
I do not found the usage in other tests in the projects。
There was a problem hiding this comment.
I don't think you understood. When you run the groovy tests in intellij then the main test is named basic statement with #connection.getClass().getCanonicalName() on #system generates spans when you expand it then you see the test with parameters. They are named basic statement with org.h2.jdbc.JdbcConnection on h2 generates spans. When you run the junit test the main test name is what you set in display name and when you expand it you'll just see the parameters. That is why I believe that using the @DisplayName they way you do does not make sense. There is no magic interpolation like in spock. You might either not add @DisplayName at all or change it to @DisplayName("basic statement generates spans")
There was a problem hiding this comment.
i will remove @DisplayName
| satisfies( | ||
| DbIncubatingAttributes.DB_USER, | ||
| val -> | ||
| val.satisfiesAnyOf( | ||
| v -> assertThat(v).isEqualTo(username), | ||
| v -> assertThat(v).isNull())), |
There was a problem hiding this comment.
the original groovy test uses
if (username != null) {
"$DbIncubatingAttributes.DB_USER" username
}
If username is given verify that this attribute exists, your assertion just makes user name optional.
There was a problem hiding this comment.
as far as I can tell you only fixed it in some places
| statement.close(); | ||
| connection.close(); |
There was a problem hiding this comment.
other tests use
cleanup.deferCleanup(statement);
cleanup.deferCleanup(connection);
There was a problem hiding this comment.
junit will autoclose connection on method parameter
I only add
cleanup.deferCleanup(statement);
There was a problem hiding this comment.
That is fine.
To me it feels a bit odd that there is a bunch of tests where connection is passes as parameter and still use cleanup.deferCleanup(connection); and there is one test that does not do it. Perhaps remove it from other tests also where it is not needed?
relate to #7195