Skip to content

Cassandra java tests#7390

Merged
trask merged 5 commits into
open-telemetry:mainfrom
aschugunov:cassandra-java-tests
Jan 18, 2023
Merged

Cassandra java tests#7390
trask merged 5 commits into
open-telemetry:mainfrom
aschugunov:cassandra-java-tests

Conversation

@aschugunov

@aschugunov aschugunov commented Dec 12, 2022

Copy link
Copy Markdown
Contributor

No description provided.

@aschugunov aschugunov requested a review from a team December 12, 2022 09:24
trask
trask reviewed Dec 13, 2022
View reviewed changes

@trask trask left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx @aschugunov!

Comment thread instrumentation/cassandra/cassandra-3.0/javaagent/src/test/java/CassandraClientTest.java Outdated
Comment on lines +93 to +100
.hasAttributesSatisfying(
attributes ->
assertThat(attributes)
.containsEntry(NET_SOCK_PEER_ADDR, "127.0.0.1")
.containsEntry(NET_SOCK_PEER_NAME, "localhost")
.containsEntry(NET_SOCK_PEER_PORT, cassandraPort)
.containsEntry(DB_SYSTEM, "cassandra")
.containsEntry(DB_STATEMENT, "USE " + parameter.keyspace))

@trask trask Dec 13, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you change the assertions with fixed number of attributes to use hasAttributesSatisfyingExactly?

@aschugunov aschugunov Dec 13, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In parameterized test some values are null and as a result attributes not exist, I didn't find a way for asserting optional parameters. Seems it is possible but it will be separate test, I mean, I should extract null values from params to new test. Is it OK ?

@aschugunov aschugunov Dec 13, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

.hasAttributesSatisfyingExactly(
      equalTo(DB_CASSANDRA_TABLE, null)
)

It works, in this case dividing is not necessary

@aschugunov aschugunov Dec 20, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@trask Could you check my changes, please?

trask
trask reviewed Jan 18, 2023
View reviewed changes

@trask trask left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aschugunov sorry for the late reply, and thanks for the help with the groovy -> Java conversion!

Comment thread instrumentation/cassandra/cassandra-3.0/javaagent/src/test/java/CassandraClientTest.java Outdated
equalTo(NET_SOCK_PEER_NAME, "localhost"),
equalTo(NET_SOCK_PEER_PORT, cassandraPort),
equalTo(DB_SYSTEM, "cassandra"),
equalTo(DB_NAME, null),

@trask trask Jan 18, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
equalTo(DB_NAME, null),

Comment thread instrumentation/cassandra/cassandra-3.0/javaagent/src/test/java/CassandraClientTest.java Outdated
Comment on lines +100 to +101
equalTo(DB_OPERATION, null),
equalTo(DB_CASSANDRA_TABLE, null))),

@trask trask Jan 18, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same, you can remove the null verifications b/c hasAttributesSatisyingExactly will fail if there are any extra attributes that you didn't verify

@aschugunov aschugunov Jan 18, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, thanks for the advice

@aschugunov aschugunov force-pushed the cassandra-java-tests branch from c1e0a3a to 664b71d Compare January 18, 2023 20:20
@aschugunov

aschugunov commented Jan 18, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

It looks like build failed due to connectivity problem and rebuild will help.

Downloading https://services.gradle.org/distributions/gradle-7.5.1-bin.zip
Error: Exception in thread "main" java.net.SocketException: An established connection was aborted by the software in your host machine
	at java.base/sun.nio.ch.NioSocketImpl.implWrite(NioSocketImpl.java:420)
	at java.base/sun.nio.ch.NioSocketImpl.write(NioSocketImpl.java:440)

trask
trask approved these changes Jan 18, 2023
View reviewed changes

@trask trask left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@trask trask enabled auto-merge (squash) January 18, 2023 21:57
@trask trask merged commit e6c2254 into open-telemetry:main Jan 18, 2023
@trask trask mentioned this pull request Feb 27, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@trask trask trask approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@aschugunov @trask