Increase the timeout period of CI to solve the unit test timeout problem of some libraries #220
Conversation
linghengqian
force-pushed
the
ci-timeout
branch
from
f0c7af8 to
63c7c82
Compare
| @@ -34,7 +34,7 @@ jobs: | |||
| test-all-metadata: | |||
| name: "🧪 ${{ matrix.coordinates }} (GraalVM ${{ matrix.versions.graalvm }} ${{ matrix.versions.java }} @ ${{ matrix.os }})" | |||
| runs-on: ${{ matrix.os }} | |||
| timeout-minutes: 20 | |||
| timeout-minutes: 80 | |||
There was a problem hiding this comment.
Why do we need to increase timeout by 4x? How long does it take to build #163 (comment) on GitHub Actions? I'd prefer if we could selectively bump the timeout for metadata that requires it.
There was a problem hiding this comment.
- Completing nativeTest for
org.apache.curator:curator-recipes:5.4.0took 62 minutes on my device. I've created the smallest possible subset of unit tests for this dependency on my local device, but the full test still takes so long.
I'd prefer if we could selectively bump the timeout for metadata that requires it.
- I don't quite understand, how to do this?
There was a problem hiding this comment.
Completing nativeTest for
org.apache.curator:curator-recipes:5.4.0took 62 minutes on my device.
That is quite a lot. Were these 64min dominated by Native Image builds or by actually running the tests?
I don't quite understand, how to do this?
Right now, we don't have a mechanism for this, but maybe we can come up with something that allows overriding the timeout if necessary. We could implement that in org.graalvm.internal.tck-harness.gradle somehow, and then use:
timeout-minutes: ${{ matrix.timeout-minutes }}There was a problem hiding this comment.
That is quite a lot. Were these 64min dominated by Native Image builds or by actually running the tests?
- When executing
./gradlew clean nativeTest, this task first executes the unit test ofcurator-recipesin a non-Native Image environment, and this process takes 29 minutes. Then the process of packaging into GraalVM Native Image took only 3 minutes, and finally executing unit tests in Native Image took 29 minutes. For reference, even directly compiling the curator-recipes component of the master branch of Apache Curator takes nearly 40 minutes.
There was a problem hiding this comment.
Do we need to run the unit tests of curator-recipes on the JVM or can we skip this step?
There was a problem hiding this comment.
- I don't think it's necessary to run unit tests on the JVM, it's definitely something to consider. But, what I see in https://graalvm.github.io/native-build-tools/latest/gradle-plugin.html#testing-support says
Currently, this feature requires the execution of the tests in the classic "JVM" mode prior to the execution of tests in native mode.
- I honestly don't see an option to disable
nativeTesttask from executing unit tests on the JVM, is there something I'm overlooking here?
There was a problem hiding this comment.
I honestly don't see an option to disable nativeTest task from executing unit tests on the JVM, is there something I'm overlooking here?
You are right, I'm afraid tests currently need to run on the JVM so that we can reliably detect all test ids.
There was a problem hiding this comment.
- So should I close this PR now?
There was a problem hiding this comment.
You can unless you want to look into making timeouts adjustable per metadata entry, which I believe we will need to support curator-recipes.
…lem of some libraries
linghengqian
force-pushed
the
ci-timeout
branch
from
63c7c82 to
743925b
Compare
What does this PR do?
org.apache.curator:curator-*:5.4.0#163 (comment) .Checklist before merging