Skip to content

Commit 236638c

Browse files
Gedochaobtomala
andauthored
Fix test scope resources to not be added to the main scope (#3898)
* Fix test scope resources to not be added to the main scope * Fix `--watch` logic to track changes to resource directories added by directives, while the resource directories' scope stays respected; add tests & refactor --------- Co-authored-by: Bartłomiej Tomala <[email protected]>
1 parent beb329d commit 236638c

File tree

5 files changed

+63
-23
lines changed

5 files changed

+63
-23
lines changed

modules/build/src/main/scala/scala/build/Build.scala

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -778,15 +778,19 @@ object Build {
778778
val watcher =
779779
new Watcher(ListBuffer(), threads.fileWatcher, run(), info.foreach(_._1.shutdown()))
780780

781-
def doWatch(): Unit = {
781+
def doWatch(): Unit = either {
782+
val (crossSources: CrossSources, inputs0: Inputs) =
783+
value(allInputs(inputs, options, logger))
782784
val elements: Seq[Element] =
783-
if res == null then inputs.elements
785+
if res == null then inputs0.elements
784786
else
785787
res
786788
.map { builds =>
789+
val allResourceDirectories =
790+
crossSources.resourceDirs.map(rd => ResourceDirectory(rd.value))
787791
val mainElems = builds.main.inputs.elements
788792
val testElems = builds.get(Scope.Test).map(_.inputs.elements).getOrElse(Nil)
789-
(mainElems ++ testElems).distinct
793+
(mainElems ++ testElems ++ allResourceDirectories).distinct
790794
}
791795
.getOrElse(inputs.elements)
792796
for (elem <- elements) {

modules/build/src/main/scala/scala/build/CrossSources.scala

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -271,19 +271,8 @@ object CrossSources {
271271
)
272272
}).flatten
273273

274-
val resourceDirectoriesFromDirectives = {
275-
val resourceDirsFromCli =
276-
allInputs.elements.flatMap { case rd: ResourceDirectory => Some(rd.path); case _ => None }
277-
val resourceDirsFromBuildOptions: Seq[os.Path] =
278-
buildOptions.flatMap(_.value.classPathOptions.resourcesDir).distinct
279-
resourceDirsFromBuildOptions
280-
.filter(!resourceDirsFromCli.contains(_))
281-
.map(ResourceDirectory(_))
282-
}
283-
val finalInputs = allInputs.add(resourceDirectoriesFromDirectives)
284-
285274
val defaultMainElemPath = for {
286-
defaultMainElem <- finalInputs.defaultMainClassElement
275+
defaultMainElem <- allInputs.defaultMainClassElement
287276
} yield defaultMainElem.path
288277

289278
val pathsWithDirectivePositions
@@ -293,7 +282,7 @@ object CrossSources {
293282
val baseReqs0 = baseReqs(d.scopePath)
294283
WithBuildRequirements(
295284
d.requirements.fold(baseReqs0)(_ orElse baseReqs0),
296-
(d.path, d.path.relativeTo(finalInputs.workspace))
285+
(d.path, d.path.relativeTo(allInputs.workspace))
297286
) -> d.directivesPositions
298287
}
299288
val inMemoryWithDirectivePositions
@@ -318,7 +307,7 @@ object CrossSources {
318307
}
319308

320309
val resourceDirs: Seq[WithBuildRequirements[os.Path]] =
321-
resolveResourceDirs(finalInputs, preprocessedSources)
310+
resolveResourceDirs(allInputs, preprocessedSources)
322311

323312
lazy val allPathsWithDirectivesByScope: Map[Scope, Seq[(os.Path, Position.File)]] =
324313
(pathsWithDirectivePositions ++ inMemoryWithDirectivePositions ++ unwrappedScriptsWithDirectivePositions)
@@ -379,7 +368,7 @@ object CrossSources {
379368
buildOptions,
380369
unwrappedScripts
381370
)
382-
crossSources -> finalInputs
371+
crossSources -> allInputs
383372
}
384373

385374
extension (uri: java.net.URI)

modules/build/src/test/scala/scala/build/tests/DirectiveTests.scala

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,21 @@ class DirectiveTests extends TestUtil.ScalaCliBuildSuite {
389389
expect(resourceDirs == Seq(path))
390390
}
391391
}
392+
test("do not include test.resourceDir into sources for main scope") {
393+
val testInputs = TestInputs(
394+
os.rel / "simple.sc" ->
395+
"""//> using test.resourceDir foo
396+
|""".stripMargin
397+
)
398+
testInputs.withBuild(baseOptions, buildThreads, bloopConfigOpt, scope = Scope.Main) {
399+
(_, _, maybeBuild) =>
400+
val build =
401+
maybeBuild.toOption.flatMap(_.successfulOpt).getOrElse(sys.error("cannot happen"))
402+
val resourceDirs = build.sources.resourceDirs
403+
404+
expect(resourceDirs.isEmpty)
405+
}
406+
}
392407
test("parse boolean for publish.doc") {
393408
val testInputs = TestInputs(
394409
os.rel / "simple.sc" ->

modules/integration/src/test/scala/scala/cli/integration/RunTestDefinitions.scala

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -567,9 +567,9 @@ abstract class RunTestDefinitions
567567
): TestInputs =
568568
TestInputs(
569569
os.rel / "src" / "proj" / "resources" / "test" / "data" -> resourceContent,
570-
os.rel / "src" / "proj" / "Test.scala" ->
570+
os.rel / "src" / "proj" / "Example.scala" ->
571571
s"""$directive
572-
|object Test {
572+
|object Example {
573573
| def main(args: Array[String]): Unit = {
574574
| val cl = Thread.currentThread().getContextClassLoader
575575
| val is = cl.getResourceAsStream("test/data")
@@ -599,6 +599,22 @@ abstract class RunTestDefinitions
599599
expect(res.out.trim() == expectedMessage)
600600
}
601601
}
602+
test("resources via test directive") {
603+
val expectedMessage = "hello"
604+
resourcesInputs(
605+
directive = "//> using test.resourceDirs ./resources",
606+
resourceContent = expectedMessage
607+
)
608+
.fromRoot { root =>
609+
val err = os.proc(TestUtil.cli, "run", ".")
610+
.call(cwd = root, check = false, stderr = os.Pipe)
611+
expect(err.err.trim().contains("java.lang.NullPointerException"))
612+
expect(err.exitCode == 1)
613+
val res = os.proc(TestUtil.cli, "run", ".", "--test")
614+
.call(cwd = root)
615+
expect(res.out.trim() == expectedMessage)
616+
}
617+
}
602618

603619
def argsAsIsTest(): Unit = {
604620
val inputs = TestInputs(

modules/integration/src/test/scala/scala/cli/integration/RunWithWatchTestDefinitions.scala

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -345,18 +345,34 @@ trait RunWithWatchTestDefinitions { _: RunTestDefinitions =>
345345

346346
for {
347347
useDirective <- Seq(false, true)
348+
testScope <- if (useDirective) Seq(false, true) else Seq(false)
349+
scopeString = if (testScope) "test" else "main"
348350
// TODO make this pass reliably on Mac CI
349351
if !Properties.isMac || !TestUtil.isCI
350-
directive = if (useDirective) "//> using resourceDirs ./resources" else ""
352+
directive =
353+
useDirective -> testScope match {
354+
case (true, true) => "//> using test.resourceDirs ./resources"
355+
case (true, false) => "//> using resourceDirs ./resources"
356+
case _ => ""
357+
}
351358
resourceOptions = if (useDirective) Nil else Seq("--resource-dirs", "./src/proj/resources")
359+
scopeOptions = if (testScope) Seq("--test") else Nil
352360
title = if (useDirective) "directive" else "command line"
353-
} test(s"resources via $title with --watch") {
361+
} test(s"resources via $title with --watch ($scopeString)") {
354362
val expectedMessage1 = "Hello"
355363
val expectedMessage2 = "world"
356364
resourcesInputs(directive = directive, resourceContent = expectedMessage1)
357365
.fromRoot { root =>
358366
TestUtil.withProcessWatching(
359-
os.proc(TestUtil.cli, "run", "src", "--watch", resourceOptions, extraOptions)
367+
os.proc(
368+
TestUtil.cli,
369+
"run",
370+
"src",
371+
"--watch",
372+
resourceOptions,
373+
scopeOptions,
374+
extraOptions
375+
)
360376
.spawn(cwd = root, stderr = os.Pipe)
361377
) { (proc, timeout, ec) =>
362378
val output1 = TestUtil.readLine(proc.stdout, ec, timeout)

0 commit comments

Comments
 (0)