You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2021/05/01 07:34:24 UTC
[GitHub] [lucene] rmuir opened a new pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
rmuir opened a new pull request #119:
URL: https://github.com/apache/lucene/pull/119
jacoco plugin is activated by `-Ptests.coverage=true`
Each parallel jvm writes in append-mode to `jacoco.db` (we clear this out before test-runs to be safe). e.g. you'll see 4 sessions if you have `tests.jvms=4`
The `jacocoTestReport` task from the jacoco plugin generates reports,
currently with defaults (HTML only).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
rmuir commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-831425688
There is a relevant jacoco issue: https://github.com/jacoco/jacoco/issues/324
It points to another issue (filtering feature), where you can see filtering asserts looks like an unimplemented TODO? So that seems to be the way forward if the yellow asserts are driving you crazy :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on a change in pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
rmuir commented on a change in pull request #119:
URL: https://github.com/apache/lucene/pull/119#discussion_r624472119
##########
File path: gradle/testing/coverage.gradle
##########
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+if (Boolean.parseBoolean(propertyOrDefault("tests.coverage", "false"))) {
+ allprojects {
+ apply plugin: "jacoco"
+ tasks.withType(Test) {
+ // XXX: too many things called "workingDir" in gradle!
+ def targetDir = "${buildDir}/tmp/tests-cwd"
Review comment:
let's keep the build clean. if you have improvements on this PR, just push.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] dweiss commented on a change in pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #119:
URL: https://github.com/apache/lucene/pull/119#discussion_r624726031
##########
File path: gradle/testing/coverage.gradle
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// This adds jacoco code coverage to tests.
+
+// Run with jacoco if either 'coverage' is passed as a task on input or
+// tests.coverage property is true.
+def withCoverage = gradle.startParameter.taskNames.contains("coverage") ||
+ Boolean.parseBoolean(propertyOrDefault("tests.coverage", "false"))
+
+if (withCoverage) {
+ allprojects {
+ plugins.withType(JavaPlugin) {
+ // Apply jacoco once we know the project has a Java plugin too.
+ project.plugins.apply("jacoco")
+
+ // Synthetic task to enable test coverage (and reports).
+ task coverage() {
+ dependsOn jacocoTestReport
+ }
+
+ // Deletes jacoco database prior to any tests.
+ task deleteCoverage(type: Delete) {
Review comment:
it was called from within test, later in the script.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-830776087
> SimpleServer is invoked from another test. It shouldn't be included by default in test runs though (there is a test file name pattern?). I'm not sure why it's being called.
The other failing test starts it as a separate process. Because of the missing permission, the main test was never working and so the issue was not seen before.
I would disable this test and fix the setup in a separate issue. The problem was there since years.
Imho, simple server should not be marked as test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] dweiss commented on a change in pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #119:
URL: https://github.com/apache/lucene/pull/119#discussion_r624465249
##########
File path: gradle/testing/coverage.gradle
##########
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+if (Boolean.parseBoolean(propertyOrDefault("tests.coverage", "false"))) {
+ allprojects {
+ apply plugin: "jacoco"
+ tasks.withType(Test) {
+ // XXX: too many things called "workingDir" in gradle!
+ def targetDir = "${buildDir}/tmp/tests-cwd"
Review comment:
I understand but it seems like it can be simplified still... If you want to merge it, go ahead and I'll take a look in the afternoon and follow-up, no problem.
##########
File path: gradle/testing/randomization/policies/tests.policy
##########
@@ -104,9 +104,16 @@ grant {
permission java.io.FilePermission "${user.home}${/}.ivy2${/}cache${/}-", "read";
permission java.io.FilePermission "${junit4.tempDir}${/}*", "read,write,delete";
permission java.io.FilePermission "${clover.db.dir}${/}-", "read,write,delete";
- permission java.io.FilePermission "${junit4.childvm.cwd}${/}jacoco.db", "write";
+};
+
+// Permissions for jacoco code coverage
+grant {
Review comment:
Oh, interesting! I didn't know about it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-830785450
I think we should at least run the test once with tests.nightly=true, as most tests in StresstestNRTReplication are only running nightly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-831408328
Jenkins "documentation": https://issues.apache.org/jira/browse/LUCENE-9188?focusedCommentId=17338040&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17338040
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-831406221
> Are our Jenkins builds publishing the Jacoco reports somewhere accessible?
Sure: https://ci-builds.apache.org/job/Lucene/job/Lucene-Coverage-main/
I described this all in issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler removed a comment on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler removed a comment on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-830703849
The security settings are still not working, aren't they?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-830704527
My latest commit should fix the replicator module. It was a bug anyways, because this missing permission silently disabled MMapDirectory.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
rmuir commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-830808504
> > Tasks configured for running with the JaCoCo agent delete the destination file for the execution data when the task starts executing. This ensures that no stale coverage data is present in the execution data.
>
> To me this does exactly what our delete task also does.
my guess is that this isn't always what gradle did :)
I put the delete task there because coverage was adding up.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-830815745
Hi @rmuir,
I removed the extra task. In my tests, the coverage was not adding up. In all combinations (with "gradlew coverage", "gradlew -Dtests.coverage=true test",...) it started with empty database and number of sessions in report was number of JVMs used for tests.
I think it's ready now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-830703824
Thanks Dawid, I changed the jenkins job to look for the other database file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-830707951
The replicator problem is not caused by MMAP not working. The missing permission made the whole TestNRTReplication.testCrashReplica test not work at all (the SimpleServer child process wasn't able to start at all because it needs sun.misc.Unsafe access to crush the JVM).
We have to fix the test or alternatively `@Ignore` it until fixed. It wasn't running correctly since years!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] dweiss commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
dweiss commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-830766980
SimpleServer is invoked from another test. It shouldn't be included by default in test runs though (there is a test file name pattern?). I'm not sure why it's being called.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on a change in pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler commented on a change in pull request #119:
URL: https://github.com/apache/lucene/pull/119#discussion_r624745614
##########
File path: gradle/testing/coverage.gradle
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// This adds jacoco code coverage to tests.
+
+// Run with jacoco if either 'coverage' is passed as a task on input or
+// tests.coverage property is true.
+def withCoverage = gradle.startParameter.taskNames.contains("coverage") ||
+ Boolean.parseBoolean(propertyOrDefault("tests.coverage", "false"))
+
+if (withCoverage) {
+ allprojects {
+ plugins.withType(JavaPlugin) {
+ // Apply jacoco once we know the project has a Java plugin too.
+ project.plugins.apply("jacoco")
+
+ // Synthetic task to enable test coverage (and reports).
+ task coverage() {
+ dependsOn jacocoTestReport
+ }
+
+ // Deletes jacoco database prior to any tests.
+ task deleteCoverage(type: Delete) {
Review comment:
But it was no problem at all. Gradle deletes the database file automatically, so o removed all that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] dweiss commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
dweiss commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-830767123
> For developers, yes. For Jenkins: No. Jenkins only needs to collect the jacoco.exec files and generates report on its own. So the jenkins job just sets property and runs tests - nothing more!
I left the property there too, just in case, so it should still work if you invoke test with property enabled.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler edited a comment on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler edited a comment on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-830706797
Jenkins works now.
Replicator module works, too; but it has 2 new test failures (looks like those 2 tests were never ran because MMAP was not working):
- org.apache.lucene.replicator.nrt.SimpleServer.classMethod (no idea why this is a test!)
- org.apache.lucene.replicator.nrt.TestNRTReplication.testCrashReplica
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler edited a comment on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler edited a comment on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-830707951
The replicator problem is not caused by MMAP not working. The missing permission made the whole `TestNRTReplication.testCrashReplica` test not work at all (the `SimpleServer` child process wasn't able to start at all because it needs sun.misc.Unsafe access to crush the JVM). Now it starts and produces a real failure, because test framework says: "this isn't a test because class name is wrong".
We have to fix the test or alternatively `@Ignore` it until fixed. It wasn't running correctly since years!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-830706797
Jenkins works now.
Replicator mdoule works, too; but it has 2 new test failures (looks like those 2 tests were never ran because MMAP was not working):
- org.apache.lucene.replicator.nrt.SimpleServer.classMethod (no idea why this is a test!)
- org.apache.lucene.replicator.nrt.TestNRTReplication.testCrashReplica
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
rmuir commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-831439579
I also tried to dig and see if I could change the `Lucene: Passing` badge in the github README.md to instead show the coverage percentage just as an easily available tracker :) I couldn't figure this out after looking for a few minutes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-831441152
> On ASF Jenkins I made the "Coverage" build to only run "daily". For testing purposes it was "hourly".
I also changed to keep last 30 *days* of builds, instead of 5 build *items*.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-830577310
Looks fine to me, but haven't tested.
I would add a job to jenkins to run all tests with jacob and publish the report as javadocs or similar (depending on plugin choices).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] dweiss commented on a change in pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #119:
URL: https://github.com/apache/lucene/pull/119#discussion_r624450212
##########
File path: gradle/testing/coverage.gradle
##########
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+if (Boolean.parseBoolean(propertyOrDefault("tests.coverage", "false"))) {
+ allprojects {
+ apply plugin: "jacoco"
Review comment:
You can specify the tool's version in the jacoco extension. I don't think it's necessary - let the plugin handle it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-831436885
> but I don't know how far it looks back (seems like only a few builds?) Maybe it just needs tweaking. If it doesn't serve our purposes, I think there is also a Rest API to fetch the data in e.g. json: https://plugins.jenkins.io/code-coverage-api/
ASF Jenkins does not seem to have the "uber" code coverage plugin with sophisticated API. It only has the "Publish Jacoco coverage" plugin (that I used). ASF Jenkins is using a Jenkins Cloudbees versions (LTS), which is stone-aged.
Policeman Jenkins is much more modern (weekly updates), so I may install a better plugin using that new API. I can play around a bit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-831428448
Another tip: Never click on methods! Just go to "class" level and then scroll down (the problem is that the many method links make you automatically click on them). Jacoco also cannot handle lambdas correctly. If you view coverage of whole class, all looks fine (except asserts).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler edited a comment on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler edited a comment on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-831406221
> Are our Jenkins builds publishing the Jacoco reports somewhere accessible?
Sure: https://ci-builds.apache.org/job/Lucene/job/Lucene-Coverage-main/
I described this all in issue. In above link, you have to select a build and there is "Test coverage" in left sidebar.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-830785124
> It shouldn't be included by default in test runs though (there is a test file name pattern?)
Although this was not the problem here: We have no filename pattern! It looks like Gradle figures tests out automatically. But this did not work anymore as soon as you enable the Java Module system. Because of this in the MMapDirectory v2 I had to add the pattern manually. We should maybe always do this, because this was my first idea: "WTF? Again the missing pattern? What the hell?"
This should IMHO be applied on main branch in both solr and lucene: https://github.com/apache/lucene-solr/pull/2176/files#diff-5389a6d358feee1c8ed596fd91b01bcb373afac3570f463c66b825401afcb297R102-R103
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on a change in pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler commented on a change in pull request #119:
URL: https://github.com/apache/lucene/pull/119#discussion_r624732263
##########
File path: gradle/testing/coverage.gradle
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// This adds jacoco code coverage to tests.
+
+// Run with jacoco if either 'coverage' is passed as a task on input or
+// tests.coverage property is true.
+def withCoverage = gradle.startParameter.taskNames.contains("coverage") ||
+ Boolean.parseBoolean(propertyOrDefault("tests.coverage", "false"))
+
+if (withCoverage) {
+ allprojects {
+ plugins.withType(JavaPlugin) {
+ // Apply jacoco once we know the project has a Java plugin too.
+ project.plugins.apply("jacoco")
+
+ // Synthetic task to enable test coverage (and reports).
+ task coverage() {
+ dependsOn jacocoTestReport
+ }
+
+ // Deletes jacoco database prior to any tests.
+ task deleteCoverage(type: Delete) {
Review comment:
This call below only added the file to the task, for deletion later. But as task never ran, nothing was done. Problem was that Delete task was not part of task chain.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
rmuir commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-830577840
@uschindler this uses defaults, which I think is HTML output only. We could attach reports as artifacts in jenkins as a start.
The plugin seems to support also CSV and XML output, and I think there are jenkins plugins that can use this structured output to generate things like graphs over time. For example, I think with python I used something to convert XML to cobertura format and used jenkins cobertura plugin to keep an eye on coverage and give fancy graphs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] dweiss commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
dweiss commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-830777822
I corrected the code - simple server actually utilizes test infrastructure and will fail if it's not a "test"... I think what I committed solves the problem with minimal effort?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on a change in pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
rmuir commented on a change in pull request #119:
URL: https://github.com/apache/lucene/pull/119#discussion_r624447305
##########
File path: gradle/testing/coverage.gradle
##########
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+if (Boolean.parseBoolean(propertyOrDefault("tests.coverage", "false"))) {
+ allprojects {
+ apply plugin: "jacoco"
Review comment:
I think it is built into gradle itself: https://docs.gradle.org/current/userguide/jacoco_plugin.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] dweiss commented on a change in pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #119:
URL: https://github.com/apache/lucene/pull/119#discussion_r624736809
##########
File path: gradle/testing/coverage.gradle
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// This adds jacoco code coverage to tests.
+
+// Run with jacoco if either 'coverage' is passed as a task on input or
+// tests.coverage property is true.
+def withCoverage = gradle.startParameter.taskNames.contains("coverage") ||
+ Boolean.parseBoolean(propertyOrDefault("tests.coverage", "false"))
+
+if (withCoverage) {
+ allprojects {
+ plugins.withType(JavaPlugin) {
+ // Apply jacoco once we know the project has a Java plugin too.
+ project.plugins.apply("jacoco")
+
+ // Synthetic task to enable test coverage (and reports).
+ task coverage() {
+ dependsOn jacocoTestReport
+ }
+
+ // Deletes jacoco database prior to any tests.
+ task deleteCoverage(type: Delete) {
Review comment:
Oh, indeed - I missed the dependsOn from the test task to that delete task, here:
https://github.com/apache/lucene/pull/119/commits/1fae3f3cb65884186e69705cba429c1a23adef39#diff-0bf04a10fde6e7b57da39affd03824f55554cbbc807f3bc09b83ac9c071a2210R51
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler edited a comment on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler edited a comment on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-830618797
Hi,
I tested with jenkins, works fine. For Jenkins it is enough to run tests with `tests.coverage=true` and configure the jenkins plugin to create the reports and save them inside jenkins. The task `jacocoTestReport` is obsolete for Jenkins.
I only had to tweak the settings a bit, because our build does not have default filenames (`*.exec`) or source location (`src/main/java`):
While running the jenkins Job, the replicator module produced a strange error with security manager:
```
> Task :lucene:replicator:test FAILED
Exception in thread "main" java.lang.reflect.InvocationTargetException
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:566)
at java.instrument/sun.instrument.InstrumentationImpl.loadClassAndStartAgent(InstrumentationImpl.java:513)
at java.instrument/sun.instrument.InstrumentationImpl.loadClassAndCallPremain(InstrumentationImpl.java:525)
Caused by: java.lang.reflect.InvocationTargetException
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:566)
at org.jacoco.agent.rt.internal_f3994fa.core.runtime.InjectedClassRuntime$Lookup.privateLookupIn(InjectedClassRuntime.java:118)
at org.jacoco.agent.rt.internal_f3994fa.core.runtime.InjectedClassRuntime.startup(InjectedClassRuntime.java:54)
at org.jacoco.agent.rt.internal_f3994fa.PreMain.premain(PreMain.java:53)
... 6 more
Caused by: java.security.AccessControlException: access denied ("java.lang.reflect.ReflectPermission" "suppressAccessChecks")
at java.base/java.security.AccessControlContext.checkPermission(AccessControlContext.java:472)
at java.base/java.security.AccessController.checkPermission(AccessController.java:897)
at java.base/java.lang.SecurityManager.checkPermission(SecurityManager.java:322)
at java.base/java.lang.invoke.MethodHandles.privateLookupIn(MethodHandles.java:189)
... 13 more
FATAL ERROR in native method: processing of -javaagent failed
```
So the replicator reports zero coverage. When this is fixed, I will disable the "Always run coverage creation, also on ABORTED or FAILED" setting. When this is merged, please remind me to change the job to use Apache Gitbox and main branch.
Here are the results of first successful run on your PR's branch:
- Main build page: https://ci-builds.apache.org/job/Lucene/job/Lucene-Coverage-main/4/
- Go to the coverage reports on left ("Testabdeckung" in German, I got crazy to find this): https://ci-builds.apache.org/job/Lucene/job/Lucene-Coverage-main/4/jacoco/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] mikemccand commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
mikemccand commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-831414606
Thanks @uschindler!
Do we disable assertions when running the Jacoco coverage? I see the `assert` statements are yellow e.g. here: https://ci-builds.apache.org/job/Lucene/job/Lucene-Coverage-main/lastCompletedBuild/jacoco/org.apache.lucene.index/IndexWriter/carryOverHardDeletes(ReadersAndUpdates,%20int,%20Bits,%20Bits,%20Bits,%20MergeState.DocMap,%20MergeState.DocMap)/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on a change in pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
rmuir commented on a change in pull request #119:
URL: https://github.com/apache/lucene/pull/119#discussion_r624452579
##########
File path: gradle/testing/coverage.gradle
##########
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+if (Boolean.parseBoolean(propertyOrDefault("tests.coverage", "false"))) {
+ allprojects {
+ apply plugin: "jacoco"
+ tasks.withType(Test) {
+ // XXX: too many things called "workingDir" in gradle!
+ def targetDir = "${buildDir}/tmp/tests-cwd"
Review comment:
@dweiss one thing is the fact that jacoco runs in append-mode. We don't want to just keep accumulating coverage over unrelated `gradle test` runs. The append-only mode serves a separate purpose, that is to allow N test jvms to append to the same output database (just saves us a merging step, really)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-830703849
The security settings are still not working, aren't they?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
rmuir commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-831442536
Thanks @uschindler . @mikemccand we could add a simple link to https://ci-builds.apache.org/job/Lucene/job/Lucene-Coverage-main/jacoco/ to luceneutil html, with Uwe's changes, it shows a graph of the last 30 days of coverage now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
rmuir commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-830829699
Thanks @uschindler @dweiss for the assistance here!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] dweiss commented on a change in pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #119:
URL: https://github.com/apache/lucene/pull/119#discussion_r624450643
##########
File path: gradle/testing/coverage.gradle
##########
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+if (Boolean.parseBoolean(propertyOrDefault("tests.coverage", "false"))) {
+ allprojects {
+ apply plugin: "jacoco"
+ tasks.withType(Test) {
+ // XXX: too many things called "workingDir" in gradle!
+ def targetDir = "${buildDir}/tmp/tests-cwd"
Review comment:
why not leave the destination file at the default value and reference it in doFirst (should work)?
```
doFirst {
project.delete jacoco.destinationFile
}
```
ah... I see. This is needed for security policy. Maybe we should pass this as a property instead? Don't know. Let me think about it.
##########
File path: gradle/testing/randomization/policies/tests.policy
##########
@@ -104,9 +104,16 @@ grant {
permission java.io.FilePermission "${user.home}${/}.ivy2${/}cache${/}-", "read";
permission java.io.FilePermission "${junit4.tempDir}${/}*", "read,write,delete";
permission java.io.FilePermission "${clover.db.dir}${/}-", "read,write,delete";
- permission java.io.FilePermission "${junit4.childvm.cwd}${/}jacoco.db", "write";
+};
+
+// Permissions for jacoco code coverage
+grant {
Review comment:
It'd be interesting to explore whether we could pass the location of individual jars (codebase) as properties and then apply these only to those jars. Something for the future. :)
##########
File path: help/tests.txt
##########
@@ -156,6 +156,16 @@ to increase the top-N count:
gradlew -p lucene/core test -Ptests.profile=true -Ptests.profile.count=100
+Generating Coverage Reports
+------------------------------
+
+Adding the property "tests.coverage=true" will run the tests with instrumentation to
+record coverage. The "jacocoTestReport" target will generate a report from those files.
+
+Example:
+
+gradlew -p lucene/core -Ptests.coverage=true test jacocoTestReport
Review comment:
I think we can automatically run jacocoTestReport (see the finalizedBy clause in plugin's documentation) if the property evaluates to true. I'll take a look.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
rmuir commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-830572404
I'm reviving this issue as I think it's an easy win to keep an eye on this coverage. Maybe I can work with @mikemccand to add reports to luceneutil, or maybe ultimately we track coverage over time via jenkins plugins.
But we've found good bugs before just looking at untested code from these reports, so I think it is good to make a start.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-831439203
On ASF Jenkins I made the "Coverage" build to only run "daily". For testing purposes it was "hourly".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
rmuir commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-830573541
Also, just casually browsing and randomly clicking the first file that looked interesting, I see huge chunks of untested code that surprised me. So I really do think the problem is just that nobody is looking at code coverage, anywhere.
For example it seems CMS's iothrottle logic is completely untested due to the fact that no merges exceed `MIN_BIG_MERGE_MB`? If you hover over `if mergeMB < MIN_MERGE_MB` statement, it explains that one of two branches are missed. so maybe this parameter should be exposed via setter at least so we can test it.
Just trying to make the argument for at least making steps to get more visibility on code coerage.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on a change in pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
rmuir commented on a change in pull request #119:
URL: https://github.com/apache/lucene/pull/119#discussion_r624457772
##########
File path: gradle/testing/randomization/policies/tests.policy
##########
@@ -104,9 +104,16 @@ grant {
permission java.io.FilePermission "${user.home}${/}.ivy2${/}cache${/}-", "read";
permission java.io.FilePermission "${junit4.tempDir}${/}*", "read,write,delete";
permission java.io.FilePermission "${clover.db.dir}${/}-", "read,write,delete";
- permission java.io.FilePermission "${junit4.childvm.cwd}${/}jacoco.db", "write";
+};
+
+// Permissions for jacoco code coverage
+grant {
Review comment:
It can work to apply permissions to jars, but only if they invoke the privileged methods using `AccessController.doPrivileged` (possibly checking their own custom permission first or something).
Given that it seems openjdk plans to deprecate SecurityManager (https://openjdk.java.net/jeps/411), I wouldn't optimize our current setup any more?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-830706001
> This runs the tests and produces a report (printing its location to the console). Seems nicer than having to remember jacoco's task names + property?
For developers, yes. For Jenkins: No. Jenkins only needs to collect the `jacoco.exec` files and generates report on its own. So the jenkins job just sets property and runs tests - nothing more!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] mikemccand commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
mikemccand commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-831398047
This looks awesome!
Are our Jenkins builds publishing the Jacoco reports somewhere accessible?
> Maybe I can work with @mikemccand to add reports to luceneutil, or maybe ultimately we track coverage over time via jenkins plugins.
+1
We could maybe also add a nightly chart of our total code coverage over time?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on a change in pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler commented on a change in pull request #119:
URL: https://github.com/apache/lucene/pull/119#discussion_r624668806
##########
File path: lucene/replicator/src/test/org/apache/lucene/replicator/nrt/TestStressNRTReplication.java
##########
@@ -607,17 +608,19 @@ NodeProcess startNode(final int id, Path indexPath, boolean isPrimary, long forc
long myPrimaryGen = primaryGen;
cmd.add("-Dtests.nrtreplication.primaryGen=" + myPrimaryGen);
+ // Mark as running nested.
+ cmd.add("-D" + TestRuleIgnoreTestSuites.PROPERTY_RUN_NESTED + "=true");
+
// Mixin our own counter because this is called from a fresh thread which means the seed
// otherwise isn't changing each time we spawn a
// new node:
long seed = random().nextLong() * nodeStartCounter.incrementAndGet();
-
cmd.add("-Dtests.seed=" + SeedUtils.formatSeed(seed));
cmd.add("-ea");
cmd.add("-cp");
cmd.add(System.getProperty("java.class.path"));
cmd.add("org.junit.runner.JUnitCore");
- cmd.add(getClass().getName().replace(getClass().getSimpleName(), "SimpleServer"));
+ cmd.add(TestSimpleServer.class.getName());
Review comment:
many thanks, this line was really a mess previously :-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] dweiss commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
dweiss commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-830687911
I've changed the code slightly so that you can just say:
```
gradlew -p lucene/core test coverage
```
or even more briefly:
```
gradlew -p lucene/core coverage
```
This runs the tests and produces a report (printing its location to the console). Seems nicer than having to remember jacoco's task names + property?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-830819657
Config now looks like this:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-830618797
Hi,
I tested with jenkins, works fine. For Jenkins it is enough to run tests with `tests.coverage=true` and configure the jenkins plugin to create the reports and save them inside jenkins. The task `jacocoTestReport` is obsolete for Jenkins.
I only had to tweak the settings a bit, because our build does not have default filenames (`*.exec`) or source location (`src/main/java`):
While running the jenkins Job, the replicator module produced a strange error with security manager:
```
> Task :lucene:replicator:test FAILED
Exception in thread "main" java.lang.reflect.InvocationTargetException
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:566)
at java.instrument/sun.instrument.InstrumentationImpl.loadClassAndStartAgent(InstrumentationImpl.java:513)
at java.instrument/sun.instrument.InstrumentationImpl.loadClassAndCallPremain(InstrumentationImpl.java:525)
Caused by: java.lang.reflect.InvocationTargetException
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:566)
at org.jacoco.agent.rt.internal_f3994fa.core.runtime.InjectedClassRuntime$Lookup.privateLookupIn(InjectedClassRuntime.java:118)
at org.jacoco.agent.rt.internal_f3994fa.core.runtime.InjectedClassRuntime.startup(InjectedClassRuntime.java:54)
at org.jacoco.agent.rt.internal_f3994fa.PreMain.premain(PreMain.java:53)
... 6 more
Caused by: java.security.AccessControlException: access denied ("java.lang.reflect.ReflectPermission" "suppressAccessChecks")
at java.base/java.security.AccessControlContext.checkPermission(AccessControlContext.java:472)
at java.base/java.security.AccessController.checkPermission(AccessController.java:897)
at java.base/java.lang.SecurityManager.checkPermission(SecurityManager.java:322)
at java.base/java.lang.invoke.MethodHandles.privateLookupIn(MethodHandles.java:189)
... 13 more
FATAL ERROR in native method: processing of -javaagent failed
```
So the replicator reports zero coverage.
Here are the results of first successful run on your PR's branch:
- Main build page: https://ci-builds.apache.org/job/Lucene/job/Lucene-Coverage-main/4/
- Go to the coverage reports on left ("Testabdeckung" in German, I got crazy to find this): https://ci-builds.apache.org/job/Lucene/job/Lucene-Coverage-main/4/jacoco/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on a change in pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler commented on a change in pull request #119:
URL: https://github.com/apache/lucene/pull/119#discussion_r624446845
##########
File path: gradle/testing/coverage.gradle
##########
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+if (Boolean.parseBoolean(propertyOrDefault("tests.coverage", "false"))) {
+ allprojects {
+ apply plugin: "jacoco"
Review comment:
Where is the version number of jacob, so builds are consistent?
Or is jacob plugin part of Gradle's bundled plugins?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-830619699
For easier setup on jenkins (maybe others may do this), let's rename the database file to `jacoco.exec` instead of `jacoco.db`. It looks like this is the standard.
_P.S.: In future we should also start to remove the outdated "ant" folder layout and be more gradle/maven conformant._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on a change in pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
rmuir commented on a change in pull request #119:
URL: https://github.com/apache/lucene/pull/119#discussion_r624454578
##########
File path: gradle/testing/coverage.gradle
##########
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+if (Boolean.parseBoolean(propertyOrDefault("tests.coverage", "false"))) {
+ allprojects {
+ apply plugin: "jacoco"
+ tasks.withType(Test) {
+ // XXX: too many things called "workingDir" in gradle!
+ def targetDir = "${buildDir}/tmp/tests-cwd"
Review comment:
just for background, this is different than the ant build, which gave each jvm its own output file, and then explicitly merged the results across the N-jvms tests files.
But I think writing to just one file is better and simpler, and it is the gradle default too. Really this is just a shutdown hook to dump the accumulated data before exiting, so it isn't like we introduce synchronization issues or performance problems.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-830790991
I found a small problem with the delete task. @dweiss: can you check?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on a change in pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
rmuir commented on a change in pull request #119:
URL: https://github.com/apache/lucene/pull/119#discussion_r624467798
##########
File path: help/tests.txt
##########
@@ -156,6 +156,16 @@ to increase the top-N count:
gradlew -p lucene/core test -Ptests.profile=true -Ptests.profile.count=100
+Generating Coverage Reports
+------------------------------
+
+Adding the property "tests.coverage=true" will run the tests with instrumentation to
+record coverage. The "jacocoTestReport" target will generate a report from those files.
+
+Example:
+
+gradlew -p lucene/core -Ptests.coverage=true test jacocoTestReport
Review comment:
Well, anything making this easier to use would be great!
I really wanted the help documentation to show two phases:
1. instrumented test execution (must be opt-in with some parameter)
2. report generation
But it made things too confusing: e.g. say you make this mistake:
```console
$ gradlew -Ptests.coverage=true test
$ gradlew jacocoTestReport
```
In that case jacoco plugin is never applied and so you get a crazy error message for missing task `jacocoTestReport`. You must also add `-Ptests.coverage=true` to this secondary reporting task, or it won't work.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-830792571
Do we need the delete task at all? From Gradle's documentation:
> Tasks configured for running with the JaCoCo agent delete the destination file for the execution data when the task starts executing. This ensures that no stale coverage data is present in the execution data.
To me this does exactly what our delete task also does.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-830788731
This all looks fine now: https://ci-builds.apache.org/job/Lucene/job/Lucene-Coverage-main/18/ -- I also ran the NRT replication tests with tests.nightly on my local machine, it all succeeded.
If nobody objects, I would like to merge this soon and reconfigure ASF jenkins to use the main branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
rmuir commented on pull request #119:
URL: https://github.com/apache/lucene/pull/119#issuecomment-831430500
> This looks awesome!
>
> Are our Jenkins builds publishing the Jacoco reports somewhere accessible?
>
> > Maybe I can work with @mikemccand to add reports to luceneutil, or maybe ultimately we track coverage over time via jenkins plugins.
>
> +1
>
> We could maybe also add a nightly chart of our total code coverage over time?
jenkins already has a little "code coverage trend": https://ci-builds.apache.org/job/Lucene/job/Lucene-Coverage-main/jacoco/
but I don't know how far it looks back (seems like only a few builds?) Maybe it just needs tweaking. If it doesn't serve our purposes, I think there is also a Rest API to fetch the data in e.g. json: https://plugins.jenkins.io/code-coverage-api/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler merged pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler merged pull request #119:
URL: https://github.com/apache/lucene/pull/119
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on a change in pull request #119: LUCENE-9188: Add jacoco code coverage support to gradle
Posted by GitBox <gi...@apache.org>.
uschindler commented on a change in pull request #119:
URL: https://github.com/apache/lucene/pull/119#discussion_r624675556
##########
File path: gradle/testing/coverage.gradle
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// This adds jacoco code coverage to tests.
+
+// Run with jacoco if either 'coverage' is passed as a task on input or
+// tests.coverage property is true.
+def withCoverage = gradle.startParameter.taskNames.contains("coverage") ||
+ Boolean.parseBoolean(propertyOrDefault("tests.coverage", "false"))
+
+if (withCoverage) {
+ allprojects {
+ plugins.withType(JavaPlugin) {
+ // Apply jacoco once we know the project has a Java plugin too.
+ project.plugins.apply("jacoco")
+
+ // Synthetic task to enable test coverage (and reports).
+ task coverage() {
+ dependsOn jacocoTestReport
+ }
+
+ // Deletes jacoco database prior to any tests.
+ task deleteCoverage(type: Delete) {
Review comment:
I am missing a dependency on this task. When is it called?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org