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

TCK Challenge: Server Async Tests #239

Open
jasondlee opened this issue Sep 29, 2024 · 8 comments
Open

TCK Challenge: Server Async Tests #239

jasondlee opened this issue Sep 29, 2024 · 8 comments

Comments

@jasondlee
Copy link
Contributor

The WildFly team would once again like to challenge the tests in JaxRsServerAsyncTest. These tests and the functionality they exercise, while perhaps useful, are not required by the MP Telemetry 2.0 specification.

The only mention of "concurrency" in the spec is to note the removal of Jakarta EE Concurrency dependency, and the only mention of "async" is vague references to "sync and async" in describing some otel properties.

The purpose of a TCK is to verify an implementation's faithfulness to a specification, and since this specification is silent -- in any meaningful way -- the TCK should be as well.

@yasmin-aumeeruddy
Copy link
Contributor

The dependency on concurrency was removed in this pull request

@donbourne
Copy link
Member

@jasondlee , can you please comment more on what you would like to see changed? As I understand, those tests are optional, and the dependency on Jakarta Concurrency was replaced with just needing an Executor service (per the above PR).

@jasondlee
Copy link
Contributor Author

@donbourne I thought it was clear: these tests are verifying something that is not required by the specification. Regardless of how the concurrency is provided (JDK vs EE), concurrency is not required by the spec. It's just not discussed anywhere in the spec at all. That being so, this test, in a suite to verify compliance with a specification is out of place and inappropriate and should be removed.

It is currently not marked as optional as best as I can tell from the spec.

@Emily-Jiang
Copy link
Member

We will discuss this challenge on the MP Telemetry call 7th October. Please attend if you can.

@jasondlee
Copy link
Contributor Author

I had an external appt this morning (and was out last week), so I was unable to attend today's meeting.

@brunobat
Copy link
Contributor

I don't mind having the test as mandatory. I was able to provide an executor (not Jakarta Concurrency) and successfully execute the test.

@jasondlee
Copy link
Contributor Author

The question, though, is are the tests appropriate according to the spec. My assertion is that they are not. Optional's an acceptable work around, I guess, but that dodges the question a bit.

@Emily-Jiang
Copy link
Member

The question, though, is are the tests appropriate according to the spec. My assertion is that they are not. Optional's an acceptable work around, I guess, but that dodges the question a bit.

It is not optional. The async part in Jakarta Restful Web Services is not optional either. Since MP Telemetry supports the usage of Jakart Restful Web Services, the async part is supported of course. There is no need to list every part of Jakarta Restful Web Services. We discussed this at length in the recent weekly calls. Please see the minutes for more info.

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

No branches or pull requests

5 participants