OpenLiberty / ci.maven

Maven plugins for managing Liberty profile servers #devops
Apache License 2.0
126 stars 89 forks source link

Maven 4 support #1674

Open yeekangc opened 1 year ago

yeekangc commented 1 year ago

Maven is working on its v4 release. There are already a few alpha releases available and a GA release is likely to come later this year. Liberty Maven plugin should support Maven 4 after it reaches GA.

Early work has been done with thanks to contribution from @bmarwell as noted in https://github.com/OpenLiberty/ci.maven/issues/1651.

bmarwell commented 1 year ago

Hey @yeekangc, I think all work for this has been committed on https://github.com/OpenLiberty/ci.maven/issues/1691. Sorry, I did not see your issue earlier.

But with warnings from 3.9.2 gone, it will automatically be working on Maven 4.

We could leave this open for optional Maven 4 github tests, though.

bmarwell commented 1 year ago

We could leave this open for optional Maven 4 github tests, though.

Ah, I would want to see https://github.com/OpenLiberty/ci.maven/issues/1653 merged first... which itself is blocked by https://github.com/OpenLiberty/ci.maven/issues/1633.

cherylking commented 1 year ago

We could leave this open for optional Maven 4 github tests, though.

Ah, I would want to see #1653 merged first... which itself is blocked by #1633.

We cannot remove dev-it test or generate-feature-test which are the two that sometimes fail. We will have to come up with a way to not gate the reference build on those two tests. Since I've managed to have several completely green builds recently, they can and do pass, but not consistently.

bmarwell commented 1 year ago

Flaky tests have negative value. They must go away. Sooner than later. If not by removing them, then by fixing them (or rewriting or whatever). I don't think removing would hurt. It failed so often in the last few months, and yet new releases were made available. How can you be sure it didn't failed because of a new bug?

Or imagine this. I create a new PR x in 15 minutes. Then I have to wait 40? Minutes on average for you to approve the test run. The test fail because they are flaky. I have to wait another 40? Minutes or so until the tests are re-run. That's not very productive, involves a lot of context switching and hinders productivity. For you and Scott, it costs real money. For my company, it costs real money the moment I start contributing in my work time.

That's why they provide negative value. They cost way more than they actually help.

"several completely green" is not good enough. It must be reliable.

Related: Some conferences mentioned "developer productivity engineering" lately.

https://devblogs.microsoft.com/engineering-at-microsoft/improving-developer-productivity-via-flaky-test-management/

https://gradle.com/blog/do-you-regularly-schedule-flaky-test-days/

https://www.atkinsondev.com/post/improve-developer-productivity/

cherylking commented 1 year ago

@bmarwell I agree with you to an extent. It absolutely is costing us money, and our contributors money, and aggravation. However, if we remove the "flaky" test, there will be no tests for dev mode. We do not know how to fix the test. It passes on the majority of the builds (e.g. it will fail on 2 out of 8 builds). It doesn't consistently fail on the same build (combination of OS, Java, OL vs WL, etc).

Perhaps we can break up the dev-it test so that we can see a pattern of a specific problem area and eliminate it. I will see what I can do.

cherylking commented 1 year ago

One more thing on flaky tests...when a Windows build fails...it is almost always due to a file locking issue which is OS specific and I have not been able to find a way to guard our tests against it. I did several things a while ago that reduced the number of times we run into it, but I do not know of a full proof solution for that.

bmarwell commented 1 year ago

Do you know the source (class) of the file lock? If it's maven, you could try the new resolver. IIRC, there were some changes. Maybe a split repo, workspace-local repo etc would help, too?

Any stack trace appreciated, I can see if I can boot up a windows VM to get behind it.

cherylking commented 1 year ago

Do you know the source (class) of the file lock? If it's maven, you could try the new resolver. IIRC, there were some changes. Maybe a split repo, workspace-local repo etc would help, too?

Any stack trace appreciated, I can see if I can boot up a windows VM to get behind it.

Thanks Ben. It is usually this file and it is related to the Liberty server itself. When the server is stopped, it should release the lock on the file. It either doesn't or it takes too much time to do so.

CWWKM2031E: Cannot delete file C:\ci.maven\liberty-maven-plugin\target\it\appsdirectory-apps-configured-it\target\liberty\usr\servers\test\workarea\.sLock. 

See this log file for a recent example. It happens on different random tests when trying to clean after the Liberty server is stopped.

cherylking commented 1 year ago

@bmarwell I refreshed the 3.8.3-SNAPSHOT this weekend so it includes the recently merged PRs.

bmarwell commented 1 year ago

@bmarwell I refreshed the 3.8.3-SNAPSHOT this weekend so it includes the recently merged PRs.

A couple of warnings are left:

$ ./mvnw -V 
Apache Maven 3.9.2 (c9616018c7a021c1c39be70fb2843d6f5f9b8a1c)
Maven home: /Users/$USER/.m2/wrapper/dists/apache-maven-3.9.2-bin/25795893/apache-maven-3.9.2
Java version: 20.0.1, vendor: IBM Corporation, runtime: /Users/$USER/Projects/sdkman/candidates/java/20.0.1-sem
Default locale: en_GB, platform encoding: UTF-8
OS name: "mac os x", version: "13.4.1", arch: "aarch64", family: "mac"

$ ./mvnw clean verify -Prun-its  -Dmaven.build.cache.enabled=false -Dmaven.plugin.validation=VERBOSE -am -pl myapp-integrationtests/openliberty

[WARNING] Plugin validation issues were detected in 4 plugin(s)
[WARNING] 
[WARNING]  * io.openliberty.tools:liberty-maven-plugin:3.8.3-SNAPSHOT
[WARNING]   Declared at location(s):
[WARNING]    * invalid.mygroupid.myapp:myapp-integrationtests-openliberty:3.0.3-SNAPSHOT (myapp-integrationtests/openliberty/pom.xml) @ line 182
[WARNING]   Used in module(s):
[WARNING]    * invalid.mygroupid.myapp:myapp-integrationtests-openliberty:3.0.3-SNAPSHOT (myapp-integrationtests/openliberty/pom.xml)
[WARNING]   Plugin issue(s):
[WARNING]    * Plugin mixes multiple Maven versions: [3.6.3, 3.5.0]
[WARNING]    * Plugin should declare these Maven artifacts in `provided` scope: [org.apache.maven:maven-plugin-api:3.5.0, org.apache.maven:maven-model:3.5.0, org.apache.maven:maven-artifact:3.6.3]
[WARNING] 
cherylking commented 1 year ago

@bmarwell Do you have a pointer on how/where to fix that? Those are all in the pom.xml as provided scope already (in liberty-maven-plugin/pom.xml).

bmarwell commented 1 year ago

@cstamas did a check and did not get the warnings. Maybe my mirror refused to update the snapshot. I'll try again from home.