kelemen / netbeans-gradle-project

This project is a NetBeans plugin able to open Gradle based Java projects. The implementation is based on Geertjan Wielenga's plugin.
173 stars 57 forks source link

Add support for "Test Files" to run all selected test classes #392

Closed tcfurrer closed 5 years ago

tcfurrer commented 6 years ago

Currently, I believe the plugin provides no way for the Netbeans user to interactively select an arbitrary set of test classes, and then use the "Test Files" operation to run all of them. When doing this, the ${selected-class} built-in variable contains only one of the selected Test classes.

Would it be possible to get the entire list of selected classes, and somehow form a single gradle cmdline with multiple --tests switches? (gradle supports this)

The only issue I see is that if too many test classes are selected, the cmdline length might quickly hit the maximum supported length. I'm not sure how to get around that issue in any way better than running groups of tests in serial (which would be a bummer since taking full advantage of gradle's parallel testing capability is important too).

Being able to run even just a handful of tests together would be very helpful for everyday development productivity. Users coming from previous ant builds are used to having this type of operation available.

This request is very close to a similar request to get true support for "Test Package", since enumerating which test classes exist in the package would be the only difference between these operations. At least today, when Gradle doesn't properly support running all tests within a specific package (see https://github.com/gradle/gradle/issues/5700 ).

tcfurrer commented 6 years ago

This request is also very close to a request to enable the "rerun failed" button in the test results pane, which currently appears disabled when running tests on gradle projects. The lack of that capability also represents a significant everyday producivity loss for Netbeans + gradle users.

kelemen commented 6 years ago

There is no command line length limit as I'm never calling something like "gradle task1 task2" but simply tell the Tooling API to run the tasks with the given argument and it sends these to the daemon in TCP message. So, there shouldn't be any limit other than the available memory (which is not an issue here).

I assume the test classes should be separated by "," but I'll check this doesn't seem that difficult to do and then I could also implement the rerun failed, since I guess this is the main thing limiting it.

tcfurrer commented 6 years ago

That's good news that this sounds feasible to you.

I don't think Gradle supports the value for the --tests argument being a comma-separated list. But I'm going to go file an Issue asking for that right now. I'm assuming that would make your job easier here. And that just makes good sense as a gradle enhancement anyway (in my opinion).

oehme commented 6 years ago

There's no need to use --tests, the Tooling API already has a dedicated API for test execution.

kelemen commented 6 years ago

@oehme What is the lowest Gradle version that feature works with? (I don't mean the Tooling API version but the Gradle daemon it connects to).

oehme commented 6 years ago

Gradle 2.6, as seen in the JavaDoc

kelemen commented 6 years ago

I probably should look at the documentation rather than lazily asking around :)

kelemen commented 6 years ago

This should be fixed now, including the re-run failed tests. You can try building the plugin from master and test if it works for you.

tcfurrer commented 6 years ago

I just tested it with Netbeans 8.2 + Gradle 4.8 + Junit 5.2.0 + Java 8 on Windows. It's pretty close to working correctly. Thanks for getting to this one so quickly!

I had some pre-existing profiles that were making use of "selected-class", and I had to update those to make use of "test-classes-args". Of course that will have to be documented.

Problem #1: For "Test Files" with multiple test classes selected, the generated gradle command has the test canonical paths appearing with a wildcard immediately after the test class name. This is both unnecessary and dangerous. It is unnecessary because gradle natively supports a canonical path to a test class, and interprets that to run all test methods within that class. It is dangerous because in the event that one test's class name is a subset of another test's name, the wildcard might match on something the user didn't select. For example, if I have two tests named ATest and ATestWithStrangeName, if the selection includes "ATest", it would also run "ATestWithStrangeName". I know this is unlikely since most projects follow the convention of suffixing all tests with "Test" and rarely include that string elsewhere in the test class names, but that is not something the plugin should make assumptions about.

I think test-classes-stared-args probably doesn't need to exist, and test-classes-args is good enough. But maybe you did this for compatability with older gradle versions, from before the class name support was added? In that case, the fix would be to add a dot-wildcard at the end instead of just a wildcard. That extra dot will ensure that it can only match on methods in the exact class desired.

Problem #2: For "re-run failed", I see the correct set of failing tests in the generated gradle command, but the test methods appear with "()" at the end (i.e. "path.to.my.test()" instead of "path.to.my.test"), which causes gradle to fail with "No tests found for given includes".

kelemen commented 6 years ago
kelemen commented 6 years ago

Actually, I think I was wrong on problem 2. NB must get the name from what I get from JUnit's test report. So, my guess is that for some reason, your test report xml contains "()" in the test name. Can you confirm that?

tcfurrer commented 6 years ago

Yes, it's because I'm using Junit 5. The Junit 5 XML files are similar to Junit 4, but there are a few important differences, and this is one of them. With Junit 5, test methods are allowed to have arguments, which probably results in the need to show the arguments in the report in order for it to be unambiguous in the face of overloaded methods.

I wouldn't be surprised if there are a few other gotchas with Junit 5 that might also require a few tweaks in the plugin. (If I run into any others, of course I'll file an issue.)

kelemen commented 6 years ago

I have fixed this buy cutting the test name at the first occurence of '(' or '[', I think that should be enough. I tried this using JUnit 5.2.0 and it works for me. I hope you can verify this too.

tcfurrer commented 6 years ago

I attempted to re-test this just now, but I couldn't open projects using the plugin built from latest source. In the "open project" window, my gradle project folders didn't appear as gradle projects despite having build.gradle files present. I did everything the same as previously, when I had no trouble downloading/building/installing/using the plugin from github. I tried a second time, this time very carefully stopping the gradle daemon first, clearing gradle and netbeans caches, etc, and that made no difference.

This attempt was on Windows, and I tried with both Netbeans 8.2 and Apache Netbeans 9 rc1.

Any ideas why the plugin didn't recognize my gradle projects? (which definitely aren't sitting in temp folders)

kelemen commented 6 years ago

I would guess that NB thinks it is another type of project (NB does not support multiple project types / directory). Otherwise, this plugin just checks for the presence of a build.gradle (or some other files). See NbGradleProjectFactory.isProject. I cannot imagine that any change I did could have caused this.

kelemen commented 6 years ago

Just remembered that I do not allow opening projects from the temp directory (System.getProperty("java.io.tmpdir")) because NB regularly puts opened files there, including build.gradle and then tries to query its class path (which will cause a project load which will most likely fail).

tcfurrer commented 6 years ago

Right, my project is not sitting under java.io.tmpdir, so that's not the problem. I did everything the same as the first time when I built the plugin and had no problem installing it. I wonder if there is a problem with the second install... like maybe when I uninstall and reinstall, something bogus is happening? Anyway, if this doesn't trace to any change you made recently, then I'll just have to debug it. If I figure out how to get it working again, I'll re-test your fixes for this issue and report back here.

tcfurrer commented 5 years ago

Now that 2.0.1 is released, I was finally able to update to the new code and successfully open projects to test these fixes.

The "rerun failed" fixes are working nicely now!

I can't remember exactly how I got the test-classes-args working before. When I try it now, I always get a stacktrace from Gradle because the entire set of arguments is passed across the API as if it were a single argument, and Gradle isn't happy with that.

I see on stdout something like: (notice there is no comma after --tests)

Arguments: [--tests mypackage.MyTest, -c, /myproject/settings.gradle, --init-script, /myproject/netbeans-init.gradle]

And the Gradle stacktrace contains something like: (notice the comma after --tests does appear here)

Caused by: org.gradle.tooling.internal.protocol.exceptions.InternalUnsupportedBuildArgumentException: Problem with provided build arguments: [--tests, mypackage.MyTest, -c, /myproject/settings.gradle, --init-script, /myproject/netbeans-init.gradle]. 
    Unknown command-line option '--tests'.

In .nb-gradle-properties, I have: (is this wrong?)

    <task>
      <display-name>test.single</display-name>
      <non-blocking>no</non-blocking>
      <task-names>
        <name must-exist="no">${project}:cleanTest</name>
        <name must-exist="no">${project}:test</name>
      </task-names>
      <task-args>
        <arg>${test-classes-args}</arg>
      </task-args>
      <task-jvm-args/>
    </task>
kelemen commented 5 years ago

tl;dr: As odd as it might seem, you have to put ${test-classes-args} in the task list not the argument list.

What gets printed is actually not what is happening (I guess I might have to change this printing code to reflect reality). There are all kinds of adjustments:

I wonder if I should just give up the silly hack in the code, and just risk the method which - I believe - is always supposed to work.

tcfurrer commented 5 years ago

Indeed, this worked beautifully:

    <task>
      <display-name>test.single</display-name>
      <non-blocking>no</non-blocking>
      <task-names>
        <name must-exist="no">${project}:cleanTest</name>
        <name must-exist="no">${project}:test</name>
        <name must-exist="no">${test-classes-args}</name>
      </task-names>
      <task-args/>
      <task-jvm-args/>
    </task>

So I believe this ticket can be closed now. Thanks so much!

( By the way, I don't think it matters much, but just for the record I tested this using: Linux, Java 8, Netbeans 9.0, Gradle 4.8 )

( One final aside: Now the only big thing I'm still waiting for is Junit 5 support in Netbeans, so that "run focused test method" starts working. Not having that functionality is really annoying, but everything else about running tests is working great now. )

kelemen commented 5 years ago

Yes, I don't think I need to do anything about it once NB supports it (I'm guessing, the main thing is to simply recognize the test annotations).