-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Scala.js: Add neg tests for bad uses of reflect.Selectable
.
#9611
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
sjrd
merged 2 commits into
scala:master
from
dotty-staging:sjs-neg-tests-infrastructure
Aug 21, 2020
Merged
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
40 changes: 40 additions & 0 deletions
40
compiler/test/dotty/tools/dotc/ScalaJSCompilationTests.scala
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
package dotty | ||
package tools | ||
package dotc | ||
|
||
import org.junit.{ Test, BeforeClass, AfterClass } | ||
import org.junit.experimental.categories.Category | ||
|
||
import scala.concurrent.duration._ | ||
import vulpix._ | ||
|
||
@Category(Array(classOf[ScalaJSCompilationTests])) | ||
class ScalaJSCompilationTests extends ParallelTesting { | ||
import ParallelTesting._ | ||
import TestConfiguration._ | ||
import ScalaJSCompilationTests._ | ||
import CompilationTest.aggregateTests | ||
|
||
// Test suite configuration -------------------------------------------------- | ||
|
||
def maxDuration = 60.seconds | ||
def numberOfSlaves = 5 | ||
def safeMode = Properties.testsSafeMode | ||
def isInteractive = SummaryReport.isInteractive | ||
def testFilter = Properties.testsFilter | ||
def updateCheckFiles: Boolean = Properties.testsUpdateCheckfile | ||
|
||
// Negative tests ------------------------------------------------------------ | ||
|
||
@Test def negScalaJS: Unit = { | ||
implicit val testGroup: TestGroup = TestGroup("negScalaJS") | ||
aggregateTests( | ||
compileFilesInDir("tests/neg-scalajs", scalaJSOptions), | ||
).checkExpectedErrors() | ||
} | ||
} | ||
|
||
object ScalaJSCompilationTests { | ||
implicit val summaryReport: SummaryReporting = new SummaryReport | ||
@AfterClass def cleanup(): Unit = summaryReport.echoSummary() | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
import scala.reflect.{ClassTag, Selectable => ReflectSel} | ||
import ReflectSel.reflectiveSelectable | ||
|
||
object Test { | ||
/* Make sure that an explicit desugaring of the legit cases actually compiles, | ||
* ensuring that the error cases we test are actually testing the right things. | ||
*/ | ||
def sanityCheck(): Unit = { | ||
val receiver: Any = ??? | ||
reflectiveSelectable(receiver).selectDynamic("foo") // OK | ||
reflectiveSelectable(receiver).applyDynamic("foo")() // OK | ||
reflectiveSelectable(receiver).applyDynamic("foo", ClassTag(classOf[String]), ClassTag(classOf[List[_]]))("bar", Nil) // OK | ||
} | ||
|
||
def badReceider(): Unit = { | ||
val receiver: ReflectSel = ??? | ||
receiver.selectDynamic("foo") // error | ||
receiver.applyDynamic("foo")() // error | ||
} | ||
|
||
def nonLiteralMethodName(): Unit = { | ||
val receiver: Any = ??? | ||
val methodName: String = "foo" | ||
reflectiveSelectable(receiver).selectDynamic(methodName) // error | ||
reflectiveSelectable(receiver).applyDynamic(methodName)() // error | ||
} | ||
|
||
def nonLiteralClassTag(): Unit = { | ||
val receiver: Any = ??? | ||
val myClassTag: ClassTag[String] = ClassTag(classOf[String]) | ||
reflectiveSelectable(receiver).applyDynamic("foo", myClassTag, ClassTag(classOf[List[_]]))("bar", Nil) // error | ||
} | ||
|
||
def classTagVarArgs(): Unit = { | ||
val receiver: Any = ??? | ||
val classTags: List[ClassTag[_]] = List(ClassTag(classOf[String]), ClassTag(classOf[List[_]])) | ||
reflectiveSelectable(receiver).applyDynamic("foo", classTags: _*)("bar", Nil) // error | ||
} | ||
|
||
def argsVarArgs(): Unit = { | ||
val receiver: Any = ??? | ||
val args: List[Any] = List("bar", Nil) | ||
reflectiveSelectable(receiver).applyDynamic("foo", ClassTag(classOf[String]), ClassTag(classOf[List[_]]))(args: _*) // error | ||
} | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I run
dotty-compiler/run
, the bootstrappedJS library is now compiled whereas it wasn't before, this would slow down working in dotty in most cases where we're not working on scala.js support. Similarly, I should be able to rundotty-compiler/testCompilation tests/pos/foo
without having to compile anything scala.js-related. So I suggest moving this stuff to another project, either an existing scala.js project like sjsJUnitTests or a new one as you prefer.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It really doesn't slow anything down, because a) it gets compiled in parallel with
dotty-library-bootstrapped
and b) it takes roughly the same time. I see a difference of about 1 out of 9 seconds on my computers (my Windows at home and my Linux at work agree) betweena)
dotty-library-bootstrapped/clean
followed bydotty-compiler/packageAll
, andb)
dotty-library-bootstrapped/clean
anddotty-library-bootstrappedJS/clean
followed bydotty-compiler/packageAll
.The alternative is to replicate a lot of infrastructure in a different project. There is currently no other project equipped to do that. This seems like a huge increase in complexity to avoid 1 second.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dotty-compiler-bootstrapped runs the regular tests + bootstrapped-only and staging-only tests, it works by overriding the
packageAll
task. You could either do the same in a different project or add the scalajs stuff to dotty-compiler-bootstrapped too, that way it doesn't impact non-bootstrapped tasks which would be acceptable.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright. Adding to the bootstrapped tests only was not an option for me, because that makes my dev cycle much much longer, since I would have to rebuild the bootstrapped compiler for every single change.
I added a separate project instead.