zyxist / chainsaw

Gradle plugin: adds support for building Java 9 modules.
Apache License 2.0
71 stars 4 forks source link

Add support for TestNG #25

Closed TheMrMilchmann closed 6 years ago

TheMrMilchmann commented 6 years ago

This is a rudimentary attempt to add TestNG support to the plugin. Most of this stuff has been straightup copied and adjusted from existing Junit support. Additionally, test packages may now be exported to all test engine modules. (See JavaModule change). This is necessary for TestNG to run due to how classes are initialized.

Unfortunately I do not have as much time to dig into the source of this plugin as I'd like, but I hope that this PR (even if this does not get merged) serves as a basis for TestNG support.

codecov-io commented 6 years ago

Codecov Report

Merging #25 into master will decrease coverage by 0.08%. The diff coverage is 90.9%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #25      +/-   ##
============================================
- Coverage     95.26%   95.17%   -0.09%     
- Complexity      199      207       +8     
============================================
  Files            34       35       +1     
  Lines           528      539      +11     
  Branches         20       20              
============================================
+ Hits            503      513      +10     
  Misses           13       13              
- Partials         12       13       +1
Impacted Files Coverage Δ Complexity Δ
.../main/java/com/zyxist/chainsaw/ChainsawPlugin.java 96.07% <ø> (ø) 14 <0> (ø) :arrow_down:
...om/zyxist/chainsaw/tests/TestTaskConfigurator.java 100% <100%> (ø) 6 <1> (+1) :arrow_up:
src/main/java/com/zyxist/chainsaw/JavaModule.java 100% <100%> (ø) 11 <2> (+2) :arrow_up:
...rc/main/java/com/zyxist/chainsaw/tests/TestNG.java 75% <75%> (ø) 4 <4> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 6bbe67a...7311774. Read the comment docs.

zyxist commented 6 years ago

Thanks for your contribution! It looks good and I don't think there will be much more to do, unless TestNG implements something unusual :).

I'd suggest two improvements:

  1. JavaModule.getExtraTestPackages() and JavaModule.setExtraTestPackages() - I think the better name would be getExportedTestPackages() and setExportedTestPackages() to clearly indicate that we are going to export them.
  2. Can you write an integration test for exporting test packages as well? These tests are very important during development, and without a good coverage the project will stuck. To generate the exportedTestPackages flags in the test build scripts, you will need to modify JigsawProjectBuilder.java - the class is not a masterpiece of engineering, but it works :).
TheMrMilchmann commented 6 years ago
  1. I have changed the names as suggested.
  2. I noticed that this is only required because my tests are located in a a package com.example.test whereas the main classes are in com.example. I still find the proposed change useful, since it creates more freedom for test package names. Furthermore, I have not added integration tests yet and will not be able to do so at least until tomorrow. However, as I'm unfamiliar with Groovy, I'd appreciate you writing the integration test (if you have some spare time).
zyxist commented 6 years ago

No rush. JigsawProjectBuilder class is written in Java, and basic Groovy is very similar to it. Take a look at existing test cases, and you will know everything you need.