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

revert unnecessary breaking change in testkit #1275

Merged
merged 1 commit into from
Nov 4, 2020

Conversation

bjaglin
Copy link
Collaborator

@bjaglin bjaglin commented Nov 4, 2020

No description provided.

bjaglin referenced this pull request Nov 4, 2020
*except in test where we compile small virtual files
@bjaglin bjaglin force-pushed the revert-breaking-change branch from b3688a8 to 0bb614f Compare November 4, 2020 22:10
@mlachkar
Copy link
Collaborator

mlachkar commented Nov 4, 2020

My change was not that clean. I can fix tests here if you want.

@bjaglin bjaglin force-pushed the revert-breaking-change branch from 0bb614f to 29b2dc7 Compare November 4, 2020 22:19
Comment on lines +222 to +223
fullClasspath.in(testsInput, Compile).value.map(_.data) :+
semanticdbTargetRoot.in(testsInput, Compile).value
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

similar strategy as in scalacenter/sbt-scalafix@cd9efb1

@@ -170,7 +170,7 @@ class CliSemanticSuite extends BaseCliSuite {
checkSemantic(
name = "-P:semanticdb:targetroot",
args = {
val (targetroot :: classDir :: Nil, jars) =
val (_ :: targetroot :: Nil, jars) =
Copy link
Collaborator Author

@bjaglin bjaglin Nov 4, 2020

Choose a reason for hiding this comment

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

not new to this PR, but the fact that order matters here shows that the test a bit fragile - I think it's acceptable as we don't put any ordering constraint for (external) testkit client, this is just a shortcut for fixturing, not a breaking change.

@bjaglin bjaglin merged commit 199b1d5 into scalacenter:master Nov 4, 2020
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.

3 participants