Skip to content

Conversation

@zeitlinger
Copy link
Member

@zeitlinger zeitlinger commented May 16, 2024

port 8080 was blocked in all spring smoke tests - breaking parallel builds

@zeitlinger zeitlinger self-assigned this May 16, 2024
@zeitlinger zeitlinger requested a review from a team May 16, 2024 20:23
@github-actions github-actions bot added the test native This label can be applied to PRs to trigger them to run native tests label May 16, 2024
@zeitlinger zeitlinger added this to the v2.4.0 milestone May 16, 2024
Comment on lines -26 to +44
assertClient(OtelSpringStarterSmokeTestController.REST_CLIENT);
testing.clearAllExportedData();

RestClient client = restClientBuilder.baseUrl("http://localhost:" + port).build();
assertThat(
client
.get()
.uri(OtelSpringStarterSmokeTestController.PING)
.retrieve()
.body(String.class))
.isEqualTo("pong");
assertClient();
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure the purpose of this test now since the RestClient telemetry is no longer captured or verified

Copy link
Member Author

Choose a reason for hiding this comment

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

assertClient is doing the verification

Copy link
Member Author

Choose a reason for hiding this comment

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

would it help to inline the method?

Copy link
Member

Choose a reason for hiding this comment

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

oh, sorry! I got confused b/c I was thinking of the other smoke test pattern where the test code itself doesn't get instrumented, only the app code, but realize that's not the case for these smoke tests

Comment on lines 175 to +181
void restTemplate() {
assertClient(OtelSpringStarterSmokeTestController.REST_TEMPLATE);
}

protected void assertClient(String url) {
testing.clearAllExportedData();

testRestTemplate.getForObject(url, String.class);
RestTemplate restTemplate = restTemplateBuilder.rootUri("http://localhost:" + port).build();
restTemplate.getForObject(OtelSpringStarterSmokeTestController.PING, String.class);
assertClient();
}
Copy link
Member

Choose a reason for hiding this comment

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

similar question here

Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

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

👍

@trask trask merged commit bbfe5a6 into open-telemetry:main May 17, 2024
@zeitlinger zeitlinger deleted the port-blocked branch May 17, 2024 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test native This label can be applied to PRs to trigger them to run native tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants