Skip to content
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

changing depedencies and build plan to be able to use parallel buld or mvnd and to be sure artifacts are available when running its use verifier in auto mode (e.g embedded most of the time) or mvnd and use verifier in auto mode (e.g embedded most of the time) #479

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

olamy
Copy link
Member

@olamy olamy commented Feb 27, 2022

  • try to include those dependencies to fix the build plan and be sure its are runned last when using -Txx or mvnd
  • use verifier in auto mode to have its running faster

Following this checklist to help us incorporate your
contribution quickly and easily:

  • Make sure there is a JIRA issue filed
    for the change (usually before you start working on it). Trivial changes like typos do not
    require a JIRA issue. Your pull request should address just this issue, without
    pulling in other changes.
  • Each commit in the pull request should have a meaningful subject line and body.
  • Format the pull request title like [SUREFIRE-XXX] - Fixes bug in ApproximateQuantiles,
    where you replace SUREFIRE-XXX with the appropriate JIRA issue. Best practice
    is to use the JIRA issue title in the pull request title and in the first line of the
    commit message.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Run mvn clean install to make sure basic checks pass. A more thorough check will
    be performed on your pull request automatically.
  • You have run the integration tests successfully (mvn -Prun-its clean install).

If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.

To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.

@olamy olamy added the build label Feb 28, 2022
@Tibor17
Copy link
Contributor

Tibor17 commented Mar 1, 2022

@olamy
This does not make sense.
First of all the Maven should have very stable Resolver avoiding file locking in the Maven local repo. I used ParallelParameterized tests, I can do it again, but due to the Maven 3.2.x locks artifacts, ParallelParameterized won't be used.

@olamy
Copy link
Member Author

olamy commented Mar 1, 2022

@olamy This does not make sense. First of all the Maven should have very stable Resolver avoiding file locking in the Maven local repo. I used ParallelParameterized tests, I can do it again, but due to the Maven 3.2.x locks artifacts, ParallelParameterized won't be used.

Dear @Tibor17
There is nothing to do with Resolver or whatever.
As explained in the description the goal is to be able to use mvn -Tx or mvnd to have a faster build (some modules can definitely be build in parallel). But doing that surefire-its is build from the start as it doesn't have any dependencies declared to the modules it will test! Which definitely doesn't reflect what the module needs to be build first.
As far I can tell the surefire-its test modules which test the maven-surefire-plugin and maven-failsafe-plugin definitely depends on the plugins to be build first.

@slawekjaranowski
Copy link
Member

@olamy
Copy link
Member Author

olamy commented Mar 2, 2022

related issue https://issues.apache.org/jira/browse/SUREFIRE-1956

definitely related

@Tibor17

just run mvn clean verify -pl :surefire-its -am and you will see dependencies are not declared correctly

with master

➜  maven-surefire git:(master) mvn clean verify -pl :surefire-its -am
[INFO] Scanning for projects...
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Build Order:
[INFO] 
[INFO] Apache Maven Surefire                                              [pom]
[INFO] Maven Surefire Integration Tests                                   [jar]
[INFO] 
[INFO] -----------------< org.apache.maven.surefire:surefire >-----------------
[INFO] Building Apache Maven Surefire 3.0.0-M6-SNAPSHOT                   [1/2]
[INFO] --------------------------------[ pom ]---------------------------------

with this branch:

➜  maven-surefire git:(multithread-build-possible) mvn clean verify -pl :surefire-its -am                    
[INFO] Scanning for projects...
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Build Order:
[INFO] 
[INFO] Apache Maven Surefire                                              [pom]
[INFO] Surefire Shared Utils                                              [jar]
[INFO] SureFire Logger API                                                [jar]
[INFO] SureFire API                                                       [jar]
[INFO] Surefire Extensions API                                            [jar]
[INFO] Surefire Extensions SPI                                            [jar]
[INFO] SureFire Booter                                                    [jar]
[INFO] Maven Surefire Common                                              [jar]
[INFO] Surefire Report Parser                                             [jar]
[INFO] Maven Surefire Plugin                                     [maven-plugin]
[INFO] Maven Failsafe Plugin                                     [maven-plugin]
[INFO] Maven Surefire Report Plugin                              [maven-plugin]
[INFO] Maven Surefire Integration Tests                                   [jar]

it is more correct but still wrong as this should include everything.

@olamy olamy force-pushed the multithread-build-possible branch from 43ab894 to 3b363aa Compare March 26, 2022 14:40
@olamy olamy marked this pull request as ready for review March 26, 2022 14:50
Copy link
Member

@slawekjaranowski slawekjaranowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea ...

First we try make possible build with test project by mvn ... verify

surefire-shared-utils is one of problem as I see ... output of shaded artifact may not be visible in reactor

surefire-its/pom.xml Outdated Show resolved Hide resolved
surefire-its/pom.xml Outdated Show resolved Hide resolved
@slawekjaranowski
Copy link
Member

From other side we don't have many boost by building project modules by many thread, at the end we land in surefire-its which consumes all time.

I thinking to use Verifier in embedded mode for it tests like in Maven core its.

@olamy
Copy link
Member Author

olamy commented Mar 26, 2022

From other side we don't have many boost by building project modules by many thread, at the end we land in surefire-its which consumes all time.

I thinking to use Verifier in embedded mode for it tests like in Maven core its.

agree but it's still something to fix as currently you could run its without running the rest of dependencies first.

@olamy olamy force-pushed the multithread-build-possible branch 2 times, most recently from 8a079d9 to 2d6cdf4 Compare June 10, 2022 04:25
@olamy olamy changed the title test hacking depedencies and build plan to be able to use -T3 or mvnd hacking depedencies and build plan to be able to use -T3 or mvnd and use verifier in auto mode (e.g embedded most of the time) Jun 10, 2022
@Tibor17
Copy link
Contributor

Tibor17 commented Jun 11, 2022

When I see the commit, I have to say that this PR is a pure mess and disaster, the providers must not be touched in the ITs POM. The POM changes have nothing to do with any build speed up, it has nothing to do with -T3. It's a pure hack which affects functionality of the tests and it is very risky solution.
As I said before in Slack or GH, the way how the build can be faster is to focus on the most slow IT classes and those are only few and those are Parameterized and they can be speed up as it was before (I deleted due to Maven Resolver) only the way that the Maven local repo would have all artifacts pre-loaded or the Resolver would be failsafe which is not now and so the concurrent ITs would fail the Resolver of course. Maiking the project build parallel would speed up the build in 5 min because all modules except the surefire-its are much faster in comparison to the ITs, so there is no reason to think about these "improvements". The penalty would wrong behavior of the tests, and that's worst!

Another story is embedded Maven. This was not stable solution for us some time ago because it shares system props and env is shared and the developer has no notion about while writing the test, and so there is the risc the IT however succeeds on Jenkins but fails in real environment. That's the reason why it was removed several years ago in the surefire-its/pom.xml.

@Tibor17
Copy link
Contributor

Tibor17 commented Jun 11, 2022

If somevody wants to keep the build order of some groupd of modules, and some other group of modules should be built in parallel, then hacking is not in place in our Apache project, that we hack ourselves. In this case you should propose a feature in Maven Core, as for instance this:

<modules concurrency="preferGivenOrder">
  <module>a</modules>
  <module parallelGroup="b">b1</modules>
  <module parallelGroup="c">c1</modules>
  <module parallelGroup="b">b2</modules>
  <module parallelGroup="c">c2</modules>
  <module>d</modules>
</modules>

@olamy olamy changed the title hacking depedencies and build plan to be able to use -T3 or mvnd and use verifier in auto mode (e.g embedded most of the time) changing depedencies and build plan to be able to use -T3 or mvnd and to be sure artifacts are available when running its use verifier in auto mode (e.g embedded most of the time) se -T3 or mvnd and use verifier in auto mode (e.g embedded most of the time) Jun 11, 2022
@olamy
Copy link
Member Author

olamy commented Jun 11, 2022

I agree hacking was a bad choice of title but it was just a quick one when first looking at it.
Maybe fixing would have been better but I updated the title to changing.

@olamy olamy force-pushed the multithread-build-possible branch from 6ecda19 to 6cdeb1f Compare June 12, 2022 09:12
@olamy
Copy link
Member Author

olamy commented Jun 12, 2022

When I see the commit, I have to say that this PR is a pure mess and disaster, the providers must not be touched in the ITs POM. The POM changes have nothing to do with any build speed up, it has nothing to do with -T3. It's a pure hack which affects functionality of the tests and it is very risky solution. As I said before in Slack or GH, the way how the build can be faster is to focus on the most slow IT classes and those are only few and those are Parameterized and they can be speed up as it was before (I deleted due to Maven Resolver) only the way that the Maven local repo would have all artifacts pre-loaded or the Resolver would be failsafe which is not now and so the concurrent ITs would fail the Resolver of course. Maiking the project build parallel would speed up the build in 5 min because all modules except the surefire-its are much faster in comparison to the ITs, so there is no reason to think about these "improvements". The penalty would wrong behavior of the tests, and that's worst!

Another story is embedded Maven. This was not stable solution for us some time ago because it shares system props and env is shared and the developer has no notion about while writing the test, and so there is the risc the IT however succeeds on Jenkins but fails in real environment. That's the reason why it was removed several years ago in the surefire-its/pom.xml.

2 facts:

  1. build plan
  • before this commits in master:
➜  maven-surefire git:(master) mvn clean install -pl :surefire-its -am
[INFO] Scanning for projects...
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Build Order:
[INFO] 
[INFO] Apache Maven Surefire                                              [pom]
[INFO] Maven Surefire Integration Tests                                   [jar]
  • after this change:
➜  maven-surefire git:(multithread-build-possible) mvn clean install -pl :surefire-its -am
[INFO] Scanning for projects...
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Build Order:
[INFO] 
[INFO] Apache Maven Surefire                                              [pom]
[INFO] Surefire Shared Utils                                              [jar]
[INFO] SureFire Logger API                                                [jar]
[INFO] SureFire API                                                       [jar]
[INFO] Surefire Extensions API                                            [jar]
[INFO] Surefire Extensions SPI                                            [jar]
[INFO] SureFire Booter                                                    [jar]
[INFO] Maven Surefire Test-Grouping Support                               [jar]
[INFO] SureFire Providers                                                 [pom]
[INFO] Shared JUnit3 Provider Code                                        [jar]
[INFO] Shared Java 5 Provider Base                                        [jar]
[INFO] Shared JUnit4 Provider Code                                        [jar]
[INFO] Shared JUnit48 Provider Code                                       [jar]
[INFO] SureFire JUnit Runner                                              [jar]
[INFO] SureFire JUnit4 Runner                                             [jar]
[INFO] Maven Surefire Common                                              [jar]
[INFO] SureFire JUnitCore Runner                                          [jar]
[INFO] SureFire JUnit Platform Runner                                     [jar]
[INFO] SureFire TestNG Utils                                              [jar]
[INFO] SureFire TestNG Runner                                             [jar]
[INFO] Surefire Report Parser                                             [jar]
[INFO] Maven Surefire Plugin                                     [maven-plugin]
[INFO] Maven Failsafe Plugin                                     [maven-plugin]
[INFO] Maven Surefire Report Plugin                              [maven-plugin]
[INFO] Maven Surefire Integration Tests                                   [jar]

I tend to think the second build plan is more correct regarding to what the surefire-its wants to achieve.
the goal is not really to use only parallel as it will provide only a bit shorter build time but hey it's still an improvement.

  1. use of verifier with auto mode:

@olamy olamy changed the title changing depedencies and build plan to be able to use -T3 or mvnd and to be sure artifacts are available when running its use verifier in auto mode (e.g embedded most of the time) se -T3 or mvnd and use verifier in auto mode (e.g embedded most of the time) changing depedencies and build plan to be able to use parallel buld or mvnd and to be sure artifacts are available when running its use verifier in auto mode (e.g embedded most of the time) se -T3 or mvnd and use verifier in auto mode (e.g embedded most of the time) Jun 12, 2022
…its are runned last when using -Txx or mvnd

Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
…deally we should use a different local repo

Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
… in target directory

Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
@olamy olamy force-pushed the multithread-build-possible branch from 6cdeb1f to 674877f Compare July 24, 2022 11:30
Signed-off-by: Olivier Lamy <olamy@apache.org>
@olamy olamy changed the title changing depedencies and build plan to be able to use parallel buld or mvnd and to be sure artifacts are available when running its use verifier in auto mode (e.g embedded most of the time) se -T3 or mvnd and use verifier in auto mode (e.g embedded most of the time) changing depedencies and build plan to be able to use parallel buld or mvnd and to be sure artifacts are available when running its use verifier in auto mode (e.g embedded most of the time) or mvnd and use verifier in auto mode (e.g embedded most of the time) Jul 28, 2022
Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
@olamy
Copy link
Member Author

olamy commented Jul 28, 2022

grhhhhh those ITs tests are so flaky depending on external resources 😩


[2022-07-28T07:38:55.790Z] Caused by: org.apache.maven.wagon.TransferFailedException: transfer failed for https://repo.maven.apache.org/maven2/org/apache/maven/wagon/wagon-provider-api/3.3.1/wagon-provider-api-3.3.1.jar

[2022-07-28T07:38:55.790Z]     at org.apache.maven.wagon.providers.http.wagon.shared.AbstractHttpClientWagon.fillInputData (AbstractHttpClientWagon.java:1250)

[2022-07-28T07:38:55.790Z]     at org.apache.maven.wagon.providers.http.wagon.shared.AbstractHttpClientWagon.fillInputData (AbstractHttpClientWagon.java:1140)

[2022-07-28T07:38:55.790Z]     at org.apache.maven.wagon.StreamWagon.getInputStream (StreamWagon.java:126)

[2022-07-28T07:38:55.790Z]     at org.apache.maven.wagon.StreamWagon.getIfNewer (StreamWagon.java:88)

[2022-07-28T07:38:55.790Z]     at org.apache.maven.wagon.StreamWagon.get (StreamWagon.java:61)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants