Skip to content
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

Support use of arquillian REST protocol #112

Merged
merged 1 commit into from Aug 9, 2023

Conversation

Azquelt
Copy link
Member

@Azquelt Azquelt commented Aug 7, 2023

  • Ensure that requests from the arquillian framework itself are ignored by the InMemorySpanExporter
  • Remove reference to the servlet API
  • Avoid using "/" path in a test application
    • The arquillian rest protocol includes a rest resource class with @path("/"). Autodiscovery for the test application is also picking up the arquillian class. If the test application also has a resource method mapped to "/", the server will fail a request to that path.

Fixes #72

- Ensure that requests from the arquillian framework itself are ignored
  by the InMemorySpanExporter
- Remove reference to the servlet API
- Avoid using "/" path in a test application
  - The arquillian rest protocol includes a rest resource class with
    @path("/"). Autodiscovery for the test application is also picking
    up the arquillian class. If the test application also has a resource
    method mapped to "/", the server will fail a request to that path.
@Azquelt
Copy link
Member Author

Azquelt commented Aug 7, 2023

It's worth noting there are other ways to fix this problem.

We could start a test span at the start of each test without the parent context. When we query the InMemorySpanExporter, we could look only for spans with a traceId which matches the test span. However, this may make it harder to tell what has gone wrong if a test fails since the span is not found rather than found with the wrong traceId. It also makes it harder to ensure that the implementation is not accidentally making extra spans with a different traceId. In the OpenLiberty tests we get around this by logging if the InMemorySpanExporter exports a span which is not claimed by any test at app shutdown and then checking the logs, but we can't do that in the TCK.

The problem of having a test REST application pick up the Arquillian REST resource, causing a conflict if the test application has a resource mapped to "/" is very unfortunate. I'd like to see if there's anything we can do in Arquillian to avoid this happening.

We could work around it by implementing getClasses() to avoid triggering autodiscovery, but we'd need to do that consistently in every test application.

Copy link
Member

@Emily-Jiang Emily-Jiang left a comment

Choose a reason for hiding this comment

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

LGTM

@Emily-Jiang Emily-Jiang merged commit a9ed2cc into eclipse:main Aug 9, 2023
3 checks passed
yasmin-aumeeruddy pushed a commit to yasmin-aumeeruddy/microprofile-telemetry that referenced this pull request Aug 10, 2023
- Ensure that requests from the arquillian framework itself are ignored
  by the InMemorySpanExporter
- Remove reference to the servlet API
- Avoid using "/" path in a test application
  - The arquillian rest protocol includes a rest resource class with
    @path("/"). Autodiscovery for the test application is also picking
    up the arquillian class. If the test application also has a resource
    method mapped to "/", the server will fail a request to that path.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TCK cannot be run using the Arquillian REST protocol
2 participants