Skip to content

Partest tests can require a java version range#9587

Merged
lrytz merged 3 commits intoscala:2.13.xfrom
lrytz:partestMinJavaVersion
Apr 23, 2021
Merged

Partest tests can require a java version range#9587
lrytz merged 3 commits intoscala:2.13.xfrom
lrytz:partestMinJavaVersion

Conversation

@lrytz
Copy link
Copy Markdown
Member

@lrytz lrytz commented Apr 22, 2021

... using a // javaVersion: N / N+ / N - M comment in the test soruce.
The test is skipped if the java version is outside the range.

@scala-jenkins scala-jenkins added this to the 2.13.6 milestone Apr 22, 2021
@SethTisue SethTisue added the internal not resulting in user-visible changes (build changes, tests, internal cleanups) label Apr 22, 2021
@lrytz lrytz requested a review from som-snytt April 22, 2021 14:30
@lrytz lrytz force-pushed the partestMinJavaVersion branch from 2ff6744 to dee4b40 Compare April 22, 2021 14:36
@som-snytt
Copy link
Copy Markdown
Contributor

Ha, already looked at this when I woke up. Wondering if it means we can get rid of the "pragma" on the check file: control the version on the input side, not the output.

@lrytz
Copy link
Copy Markdown
Member Author

lrytz commented Apr 22, 2021

I didn't add support for java version ranges to keep it simple, but that would also be a possibility. Getting rid of the pragmas in check files would then mean duplicating the test, which doesn't gain us much I think...

@som-snytt
Copy link
Copy Markdown
Contributor

--update-check doesn't respect pragmas in check files, and probably never will.

There are only a few tests that are version or platform dependent. Maybe instead of range, just jdk: 11 17 where someone testing locally with 16 will see a skipped test, which is reasonable.

@lrytz
Copy link
Copy Markdown
Member Author

lrytz commented Apr 22, 2021

I agree that maintaining .check files with pragmas is a huge pain. Though switching jvm for a "quick" --update-check will be not so quick either :-)

I can do version ranges.

Copy link
Copy Markdown
Contributor

@som-snytt som-snytt left a comment

Choose a reason for hiding this comment

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

toolArgsForTheWin

@lrytz lrytz force-pushed the partestMinJavaVersion branch from 595f3aa to 5f8af99 Compare April 23, 2021 13:10
@lrytz
Copy link
Copy Markdown
Member Author

lrytz commented Apr 23, 2021

Changed to support java version ranges, also added some doc to CONTRIBUTING.md

lrytz added 3 commits April 23, 2021 15:14
... using a `// javaVersion: N / N+ / N - M` comment in the test soruce.
The test is skipped if the java version is outside the range.
@lrytz lrytz force-pushed the partestMinJavaVersion branch from 5f8af99 to 3baff01 Compare April 23, 2021 13:16
@lrytz lrytz marked this pull request as ready for review April 23, 2021 13:16
@lrytz lrytz changed the title Partest tests can require a minimal java version Partest tests can require a java version range Apr 23, 2021
@lrytz lrytz merged commit d112b8c into scala:2.13.x Apr 23, 2021
@harpocrates
Copy link
Copy Markdown
Contributor

Thank you so much for adding this, and so promptly too. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal not resulting in user-visible changes (build changes, tests, internal cleanups)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants