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

Add language allowing TCK modification in service release after chall… #1496

Open
wants to merge 3 commits into
base: src
Choose a base branch
from

Conversation

scottkurz
Copy link
Contributor

…enge

Signed-off-by: Scott Kurz [email protected]

@netlify
Copy link

netlify bot commented Jun 27, 2022

Deploy Preview for jakartaee ready!

Name Link
🔨 Latest commit 8ec27e4
🔍 Latest deploy log https://app.netlify.com/sites/jakartaee/deploys/62c4870540f54f0008b3ad48
😎 Deploy Preview https://deploy-preview-1496--jakartaee.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@scottkurz
Copy link
Contributor Author

Perhaps the "example" I added:

the test validation logic to "expand" the validation check.
E.g. if a test expects a value "A1" for a specific variable "x", but a challenge
is raised arguing that the specification language actually allows for either
values "A1" and "A2" (but no other values) to be valid values for "x", then
it could be a valid course of action to modify the challenged test to allow
either "A1" OR "A2" for "x".

is too verbose and unnecessary. The key point is simply that the previously-certified implementations pass the newly modified TCK, and maybe that's all that's needed to be said.

@ivargrimstad
Copy link
Member

Thanks, @scottkurz!
We should discuss this in the upcoming Specification Committee meeting to see if anyone has objections or suggested changes.

To limit the confusion and additional work such a scenario would cause, if
there is already at least one certified compatible implementation before the challenge,
the new, modified TCK should be run against at least one such implementation (and ideally all of them)
before the changes are published, released, and finalized.
Copy link
Contributor

Choose a reason for hiding this comment

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

+100 for limiting confusion + additional work but I don't see how this can currently be coordinated in a way that doesn't cause other problems.

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps we could use a specific github issue for communicating that there is a staged (or possibly later on a release milestone released to a Maven repos) that can be used to validate that a TCK change is compatible with already certificated as compatible releases.

@scottmarlow
Copy link
Contributor

I just opened scottkurz#1 in support of this change. Feedback on that is welcome unless @scottkurz wants to just merge it into this pr.

…ification-after-challenge_marlow

Add language allowing test modification after challenge marlow
@scottkurz
Copy link
Contributor Author

I just opened scottkurz#1 in support of this change. Feedback on that is welcome unless @scottkurz wants to just merge it into this pr.

Merged into this PR, thank you, Scott M.

I was also wondering about the alternative of saying if you do anything other than exclude the test you have to get approval of the Specification Committee.

But it seems the concurrency TCK issues are now driving the discussion in this area:
https://www.eclipse.org/lists/jakarta.ee-spec/msg02658.html
so maybe discussion in the current PR isn't as helpful.

@scottmarlow
Copy link
Contributor

I was also wondering about the alternative of saying if you do anything other than exclude the test you have to get approval of the Specification Committee.

But it seems the concurrency TCK issues are now driving the discussion in this area: https://www.eclipse.org/lists/jakarta.ee-spec/msg02658.html so maybe discussion in the current PR isn't as helpful.

IMO if this pull request is merged/released (with whatever additional changes are needed), that could change the answer to question #1 in https://www.eclipse.org/lists/jakarta.ee-spec/msg02658.html, so I would like to complete the TCK Process change.

@edbratt
Copy link
Contributor

edbratt commented Jul 6, 2022

No TCK should be released without at least one implementation proving compatibility so, the clause above would always be true. To me, it doesn't make sense to file a 'challenge' to an unratified TCK -- this case should simply be handled as part of the TCK version development.

I don't know how we can know if work is in progress and we should not presume that a Certification Request is completed, or even created (on can create a CCR that in incomplete, without any time limitations). Instead, I would suggest we adopt a policy of allowing for 'alternate' tests.

Once the TCK is ratified, those artifacts become a body of work that can be used for certification -- in fact, at least one certification will have been proven with it. If, at a later date, it is found that a test must be changed, the specification committer team would be allowed to provide the changed test as an alternate test. Vendors are then allowed to choose from the original or the alternate test when they perform and submit their CCR. The test result should simply indicate that the TCK Passes. I don't believe there should be a requirement to report if a vendor passed the original or alternate test. If the test, in fact changes the compatibility of the implementation, then I would suggest that this is evidence the test is not merely fixed, but is in fact changed and a specification feature update should be made.

I would recommend text such as:

In a patch release, a specification team may provide alternate tests it deems necessary. For example in a case responding to a TCK challenge. Alternate tests may only replace previously existing compatibility tests. Specification teams may not add new tests as alternates.

When alternate tests are available, a vendor may choose, at their discretion, between the original and the alternate tests when performing their compatibility certification. Alternate test results should record the test identifier and the pass/fail/skip status.

Only the final summary (count of tests pass/fail/skip) is necessary in the final TCK result documentation (provided by the vendor). The onus is on the Specification team to collaborate with the vendor team to ensure that the alternate tests are used and are performing the same compatibility function as the original tests and that the CCR represents use of the alternate tests. This collaboration is only needed for the initial release of the patch release TCK.

An implementation is certified compatible when all TCK tests pass, regardless of which tests (original or alternate) are used.

Alternate tests must be validated in the same fashion as any other TCK test. A CCR must be filed for an implementation that demonstrates a successfully passing implementation exists. The verification CCR must be filed when the TCK, with the alternate tests, is released.

@Emily-Jiang
Copy link
Contributor

One issue with alternate is that the tests might make into future release. A further clean up needs to be performed before another minor/major release. We need to track this. I am not sure whether it is necessary to introduce this approach as an implementation can choose which service release they certify. If an implementation certifies against a previous tck, they can continue to do so.

Also, as mentioned before on the mailing list, I think we should have more freedom to update tests before platform or web profile releases since they will lock down which service release they will pull in. We can fix the tcks on a particular service release and then package with platform or web profile release. Any runtime that certifies against individul specs instead web profile or platform, it can choose a particular version.

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.

5 participants