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

Bloop is no able to detect custom source sets #1765

Open
slovvik opened this issue Jul 24, 2022 · 7 comments
Open

Bloop is no able to detect custom source sets #1765

slovvik opened this issue Jul 24, 2022 · 7 comments
Labels
bug A defect or misbehaviour. sbt

Comments

@slovvik
Copy link

slovvik commented Jul 24, 2022

Given the following configuration:

lazy val SystemTest = config("system") extend(Test)

lazy val root = (project in file(".")).
configs(IntegrationTest, SystemTest)
  .settings(
    inConfig(SystemTest)(Defaults.testSettings),
    Defaults.itSettings,
    inThisBuild(List(
      organization := "com.example",
      scalaVersion := "2.13.6"
    )),
    name := "scalatest-example"
  )

libraryDependencies += "org.scalatest" %% "scalatest" % "3.2.9" % Test
libraryDependencies += "org.scalatest" %% "scalatest" % "3.2.9" % IntegrationTest
libraryDependencies += "org.scalatest" %% "scalatest" % "3.2.9" % SystemTest

and when running sbt bloopInstall the following files are generated:

[success] Generated .bloop/root.json
[success] Generated .bloop/root-test.json
[success] Generated .bloop/root-it.json

bloop projects also show only:

root
root-it
root-test

It looks like bloop is not able to detect the system-test source set.

@dos65
Copy link
Collaborator

dos65 commented Jul 25, 2022

Thanks for reporting! Yep it doesn't support non build-in scopes.

I took a quick look so here a bit details:

bloopGenerate task is defined only for Compile, Test, IntegrationTest scopes:

sbt.inConfig(Compile)(configSettings) ++
sbt.inConfig(Test)(configSettings) ++
sbt.inConfig(IntegrationTest)(configSettings) ++

And bloopInstall filter scopes where this task is defined:

def bloopInstall: Def.Initialize[Task[Unit]] = Def.taskDyn {
val filter = sbt.ScopeFilter(
sbt.inAnyProject,
sbt.inAnyConfiguration,
sbt.inTasks(BloopKeys.bloopGenerate)
)

Also, we can use this method imp from sbt-bsp as a reference: https://github.com/sbt/sbt/blob/1f29c9053cdc52128fa45cfe0b22e9a4d085cc1b/main/src/main/scala/sbt/internal/server/BuildServerProtocol.scala#L520

@slovvik
Copy link
Author

slovvik commented Jul 25, 2022

@dos65 thank you for taking a look. Is it something that can be taken by someone? :) We are frustrated with using Intellij, but without this feature, we can't switch. For me, it will take like a whole eternity to do it :(

@Arthurm1
Copy link
Contributor

@slovvik I don't know much about SBT but can you use this document's suggestion as a workaround?

Adding addSbtPlugin("ch.epfl.scala" % "sbt-bloop" % "1.5.2") to project/plugins.sbt

and using something like...

import bloop.integrations.sbt.BloopDefaults

lazy val SystemTest = config("system") extend(Test)

lazy val root = (project in file(".")).
configs(IntegrationTest, SystemTest)
  .settings(
    inConfig(SystemTest)(Defaults.testSettings ++ BloopDefaults.configSettings),
    Defaults.itSettings,
    inThisBuild(List(
      organization := "com.example",
      scalaVersion := "2.13.6"
    )),
    name := "scalatest-example"
  )

libraryDependencies += "org.scalatest" %% "scalatest" % "3.2.9" % Test
libraryDependencies += "org.scalatest" %% "scalatest" % "3.2.9" % IntegrationTest
libraryDependencies += "org.scalatest" %% "scalatest" % "3.2.9" % SystemTest

Not sure if that's what you need (or even if that's the best way to alter the SBT build).

I note that any non Test or IntegrationTest config will be tagged as a library so your system config tests won't be recognised in Metals as runnable

Perhaps the default bloopInstall should include all configs and tag them as tests if they inherit from the test config 🤷

@slovvik
Copy link
Author

slovvik commented Jul 25, 2022

@Arthurm1 it seems to do the trick but only in bloop. It has no support in the IDE, Metals in my case. It even broke current support for Test and IntegrationTest. Now I can't run them from the UI.

Perhaps the default bloopInstall should include all configs and tag them as tests if they inherit from the test config

I think so. Nowadays projects have all kinds of tests, so it would be cool to support them.

@tgodzik
Copy link
Contributor

tgodzik commented Jul 27, 2022

@dos65 looks like we could use the logic from BSP, but that would require someone with a much better knowldege of sbt than me 😅

I think so. Nowadays projects have all kinds of tests, so it would be cool to support them.

Sure, the alternative would be having them under the same scope Test but in different modules if you need to split them. However, it would be good to support the different scopes.

@dos65
Copy link
Collaborator

dos65 commented Jul 27, 2022

I will probably look at this issue a bit later.
I dropped the notes because didn't want to switch on it immediately 😄

@tgodzik tgodzik added bug A defect or misbehaviour. sbt labels Aug 17, 2022
@francesconero
Copy link

Hello, I was also interested in improving the test detection for custom configs. Digging through the code in the sbt integration I find that the tag selection logic is currently:

val tags = configuration match {
case IntegrationTest => List(Tag.IntegrationTest)
case Test => List(Tag.Test)
case _ => List(Tag.Library)
}

which, as already mentioned, doesn't take into account extended configurations.

Would changing it to something along these lines be enough?

val tags = List(
      depsFromConfig(configuration)
        .collectFirst {
          _.id match {
            case IntegrationTest.id => Tag.IntegrationTest
            case Test.id => Tag.Test
          }
        }
        .getOrElse(Tag.Library)
    )

I'm not sure about using the id of the configuration, but it would cover another (at least I think) common scenario where one simply does val myCustomIntegrationTest = IntegrationTest extend(Test). In that case going with a simple object equality comparison would only match the extended Test configuration, but I believe that to be a bit surprising, since the intention is probably just to slightly alter the default IntegrationTest behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A defect or misbehaviour. sbt
Projects
None yet
Development

No branches or pull requests

5 participants