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

fix dm hover test on j 21 #3009

Merged
merged 1 commit into from
Apr 24, 2024
Merged

fix dm hover test on j 21 #3009

merged 1 commit into from
Apr 24, 2024

Conversation

cdietrich
Copy link
Member

No description provided.

Copy link
Contributor

@LorenzoBettini LorenzoBettini left a comment

Choose a reason for hiding this comment

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

But this would work only in Java 21, not in previous versions.

@cdietrich
Copy link
Member Author

I don’t understand
See my change in reference projects

@LorenzoBettini
Copy link
Contributor

I mean that the modified test will rely only on the new Javadoc of List introduced in Java 21. In previous versions of Java it will fail because the Javadoc was different. If we're sure we change the domainmodel to require Java 21, then no problem.

Otherwise

  • we rely on a custom stubbed class with a fixed javadoc (see my other changes in Initial preparation for Java 21 #2986, which is the safest choice because we have full control)
  • we use the check for the current Java version and specify the assertions accordingly
  • we introduce a less strict method in the hover test, something like hasHoverContaining

@cdietrich
Copy link
Member Author

cdietrich commented Apr 24, 2024

i dont understand.
https://github.com/xtext/xtext-reference-projects/actions/runs/8813287543
dm is green here will all java versions

hasHoverOver is a containing check

@LorenzoBettini
Copy link
Contributor

Sorry, I missed the fact it was already a containing check.

And sorry: I thought you changed the tests to add the (also known as a <i>sequence</i>), instead you changed it to remove it because it disappeared from Java 21 javadoc.

Sorry for the noise, I now understand :)

@LorenzoBettini LorenzoBettini self-requested a review April 24, 2024 09:07
@cdietrich cdietrich merged commit fb39453 into main Apr 24, 2024
6 checks passed
@cdietrich cdietrich deleted the cd/fixDmJ21 branch April 24, 2024 09:08
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.

None yet

2 participants