scalameta / metals

Scala language server with rich IDE features 🚀
https://scalameta.org/metals/
Apache License 2.0
2.1k stars 335 forks source link

Cannot run TestNG tests. #6826

Open charlesbthomas opened 1 month ago

charlesbthomas commented 1 month ago

Describe the bug

When using my editor to try and run tests I have no code lenses for any tests using the TestNG test framework.

Furthermore, when I try to run all tests in a file, there are no tests detected.

Expected behavior

Operating system

macOS

Editor/Extension

Nvim (nvim-metals)

Version of Metals

1.3.5

Extra context or search terms

No response

tgodzik commented 1 month ago

Thanks for reporting! Do you maybe have a sample project that I could use to reproduce it?

charlesbthomas commented 1 month ago

@tgodzik I will work on putting one together!

charlesbthomas commented 1 month ago

@tgodzik Here is an example repo that replicates the issue.

https://github.com/charlesbthomas/testng-example

Screenshots:

Working code lens on the app:

Screenshot 2024-10-09 at 9 30 19 PM

No code lens on the tests:

Screenshot 2024-10-09 at 9 30 45 PM
charlesbthomas commented 1 month ago

I am not familiar with the metals codebase (just a happy user), but my hunch is that we need to implement something like:

https://github.com/scalameta/metals/blob/2c8d344da6dc8b4e920612e4336b28a493f021d0/metals/src/main/scala/scala/meta/internal/metals/testProvider/frameworks/JunitTestFinder.scala#L13

but for TestNG.

tgodzik commented 1 month ago

Sure, but without it it should still be possible to run the whole test suite.

charlesbthomas commented 1 month ago

Sure, but without it it should still be possible to run the whole test suite.

If you pull that example down, you can see that it is unable to run all the tests in that file.

tgodzik commented 1 month ago

That's what I mean, we need to fix that part fist.

charlesbthomas commented 3 weeks ago

@tgodzik any ideas on where someone might start with this? I am pretty motivated to fix it and willing to work on the fix as a contribution, but not sure where I would even start.

tgodzik commented 3 weeks ago

Looking at the frameworks available I don't think we support TestNG in Bloop. It should be possible as there are interfaces for sbt https://github.com/sbt/sbt-testng <- those same should be possible to use in Bloop, but they seem outdated.

We would probably need to add sbt-testng on the classpath for Bloop (maven classpath should be translated to the Bloop one, so we can add it there) and add new framework entry like the ones here:

https://github.com/scalacenter/bloop-config/blob/main/config/src/bloop/config/Config.scala#L13

pointing to https://github.com/sbt/sbt-testng/blob/master/src/main/scala/de/johoop/testnginterface/TestNGFramework.scala

You can experiment by publishing bloop-config locally, change it in https://github.com/scalacenter/bloop-maven-plugin , publish that locally also and try to export a project with the added sbt-testng on the classpath manually.

That should make metals discover the test suites.

You can also modify the bloop config file manually in .bloop for testing purposes after exporting with the sbt-testng added to the classpath to see if it's worth updating bloop-config.

Maybe that would be enough for the test suites to be discoverable. A bonus point would be to only add the framework if the appropriate classes are available, but it doesn't seem we do it for any other test frame work, so we should be fine without.

tgodzik commented 3 weeks ago

Let me know if you manage to test it out!

tgodzik commented 3 weeks ago

Looks like there is a newer artifact we can use from mill com.lihaoyi:mill-contrib-testng:0.12.1

charlesbthomas commented 2 weeks ago

@tgodzik I tried adding sbt-testng to my test project above's classpath and manually editing the bloop config to also include:

                {
                    "names": [
                        "de.johoop.testnginterface.TestNGFramework"
                    ]
                }

Still no luck.

tgodzik commented 1 week ago

I will have to take a look then