Skip to content

Port text block support #14882

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

Merged
merged 1 commit into from
Jun 15, 2022
Merged

Port text block support #14882

merged 1 commit into from
Jun 15, 2022

Conversation

som-snytt
Copy link
Contributor

toolargs supports multi tools

Tests need args for scalac, javac and also to specify
test conditions such as required jvm version.

Forward port of scala/bug#12290
thanks @harpocrates

@som-snytt
Copy link
Contributor Author

The jvm flag mechanism needs a tweak.

toolargs supports multi tools

Tests need args for scalac, javac and also to specify
test conditions such as required jvm version.
@som-snytt
Copy link
Contributor Author

Maybe records was going to go on this branch?

@som-snytt
Copy link
Contributor Author

It would be nice to have those toolArgs.

@smarter smarter requested a review from Kordyjan April 27, 2022 17:11
@smarter smarter assigned Kordyjan and unassigned som-snytt Apr 27, 2022
@Kordyjan Kordyjan merged commit 213fdbc into scala:main Jun 15, 2022
@som-snytt som-snytt deleted the forward/text-blocks branch June 15, 2022 16:49
@griggt griggt mentioned this pull request Jun 15, 2022
33 tasks
@mbovel
Copy link
Member

mbovel commented Jun 16, 2022

Windows tests seem to be failing since this PR. I am having a look now.

@som-snytt
Copy link
Contributor Author

I hope it is not the TODO about SCALA_ONLY and case-insensitive file systems.

@som-snytt
Copy link
Contributor Author

som-snytt commented Jun 16, 2022

Maybe it's the // test: -jvm 15+ under [info] welcome to sbt 1.6.2 (AdoptOpenJDK Java 1.8.0_265)

Looks like the jdk 8 runs fail irrespective of platform.

@som-snytt
Copy link
Contributor Author

som-snytt commented Jun 16, 2022

@mbovel I can take a look right now; or I may need help knowing how to "skip" a test under vulpix. I remember Lukas added skipping for this test feature under partest.

@mbovel
Copy link
Member

mbovel commented Jun 16, 2022

I can take a look right now

Ok, great!

I may need help knowing how to "skip" a test under vulpix

I don't know but I can have a look.

@som-snytt
Copy link
Contributor Author

I will add a skip result to vulpix...

@mbovel
Copy link
Member

mbovel commented Jun 16, 2022

I will add a skip result to vulpix...

Is the goal to temporary ignore tests?

If yes, then wouldn't just moving them to tests/disabled work?

@som-snytt som-snytt mentioned this pull request Jun 16, 2022
@som-snytt
Copy link
Contributor Author

som-snytt commented Jun 16, 2022

@mbovel sorry I missed your comment. Yes, moving the test out of the way would have worked for today, and I would have done that as a stop-gap if I hadn't missed your comment. Unfortunately, tests/disabled seems to mock disability, so it should be called tests/nonworking except that mocks the unemployed. Since I put in some time on the test rig, I will see it through; but if it doesn't merge quickly, I'll resort to the measure you suggest.

I noticed the test of scala's java parser need not required jdk 15; only the test using javac need skip on jdk 8.

// Inspect the first 10 of the given lines for compiler options of the form
// scalac: arg1 arg2, with alternative opening, optional space, alt names, text that is not */ up to end.
// groups are (name, args)
private val toolArg = raw"(?://|/\*| \*) ?(?i:(${ToolName.values.mkString("|")})):((?:[^*]|\*(?!/))*)".r.unanchored
Copy link
Member

Choose a reason for hiding this comment

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

@Kordyjan Kordyjan added this to the 3.2.0 milestone Aug 1, 2023
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.

4 participants