-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Documentation on generating test coverage is outdated #28867
Comments
No, more than a documentation update is needed here, the Jacoco coverage is not even configured into the build.
Are you offering to contribute all the work needed here? |
FYI @elastic/es-core-infra. |
Ahh okay. Sure, I'll be able to contribute. Seeing as I'll have to rummage around in the build configuration, is there any specific place you would like me to place the JaCoCo configuration? Or anything else I should keep in consideration? |
Thanks a lot! Take a look at our |
Feel free to ping me as a reviewer as well @bvobart , ill run it thru the paces locally once you get it pushed up |
@jasontedor @hub-cap I'm having quite some problems getting JaCoCo working (or Cobertura for that matter). Having tried adding JaCoCo in multiple different ways without success, I finally found that It turns out that the JaCoCo plugin looks for all tasks with type While Click to expand error message
Do you have any idea how we could fix this? Or work around this in some other way? If you want to see the changes I made to the build configuration, see the jacoco branch on my fork here. |
I originally tried to make RandomizedTestingTask extend Test, but there were many issues with it (I don't remember if they were exactly the same as you found here). I suggest calling jacoco in ant directly. |
Hmm okay. Personally, I believe that it should be a goal to have While it might be slightly off-topic, I do want to ask: how do you as main Elastic developers currently get insight in test coverage? |
We have no control over that really. I don't think gradle has any desire to make Test extendable. As I said before, I did start there when creating RandomizedTestingTask.
We should have a new task in buildSrc under org.elasticsearch.gradle.precommit which is then setup in PrecommitTasks.
Currently, I run these in Intellij for specific test classes when I want to ensure something is covered. |
Actually, Gradle's
I took a look here and there are several reasons why this either will not work, or why you should not want it there. The most important reason is that JaCoCo requires that the tests are run before it can generate its coverage reports, so you'd have to run the tests during the precommit tasks. Even then, I doubt whether you wouldn't run into the same issue with For the mean time, I'll create a PR updating the documentation on generating test coverage to reflect that it is currently only really possible through IntelliJ, linking to this issue. |
Closed by #29255 |
The TESTING documentation states that test coverage can be generated with a Maven command. However, Elasticsearch currently uses Gradle. Issue #13930 describes the migration from Maven to Gradle and states that the documentation for building and testing was completely updated except for the documentation on JaCoCo reports.
So is it currently possible to create a test coverage report? And if so, how? I'd gladly open a PR with updated documentation once I get it working.
The text was updated successfully, but these errors were encountered: