OpenLiberty / liberty-tools-intellij

IntelliJ IDEA extension for Liberty
https://plugins.jetbrains.com/plugin/14856-open-liberty-tools
Eclipse Public License 2.0
14 stars 26 forks source link

Investigate time out waiting for test report #934

Open TrevCraw opened 2 months ago

TrevCraw commented 2 months ago

The builds started consistently failing for each platform. We need to investigate what is causing this.

For Linux and Windows, it looks like each of the failures are coming from tests that attempt to run project tests through dev mode and hit this error:

java.lang.IllegalStateException: Timed out waiting for test report: /home/runner/work/liberty-tools-intellij/liberty-tools-intellij/liberty-tools-intellij/src/test/resources/projects/maven/singleModMavenMP/target/site/failsafe-report.html file to be created.
        at io.openliberty.tools.intellij.it.TestUtils.validateTestReportExists(TestUtils.java:396)

Refer to this workflow run: https://github.com/OpenLiberty/liberty-tools-intellij/actions/runs/10590421767

This workflow also hit this error in the Linux builds: https://github.com/OpenLiberty/liberty-tools-intellij/actions/runs/10592982820. It should be noted that the PR for this workflow is targeting the feature branch, and the PR for the workflow above is targeting the main branch.

I also kicked off a manual workflow on the main branch: https://github.com/OpenLiberty/liberty-tools-intellij/actions/runs/10598016842. All the test failures for Linux and Windows are the test report timeouts.


Related

turkeylurkey commented 2 months ago

After we click on Run Tests the system starts many dozens of Maven downloads but these proceed quickly on Linux and the tests seem to run correctly right after. image After clicking on Run Tests the process waits 100s for the test files to appear. This seems like it should be enough so we should see if the result files are going somewhere else instead of target/site. Possibly the file names changed for some reason.

turkeylurkey commented 2 months ago

In a linux test the log shows what files are being examined:

MavenSingleModMPProjectTest > testRunTestsActionUsingSearch() STANDARD_OUT
    INFO: 2024-08-28T16:22:00.675403834: MavenSingleModMPProjectTest.testRunTestsActionUsingSearch(). Entry
    INFO: 2024-08-28T16:22:00.680408280: deleteFile. Entry. Path: /home/runner/work/liberty-tools-intellij/liberty-tools-intellij/liberty-tools-intellij/src/test/resources/projects/maven/singleModMavenMP/target/site/failsafe-report.html
    INFO: 2024-08-28T16:22:00.680448806: deleteFile. Exit. Deleted: true
    INFO: 2024-08-28T16:22:00.680725534: deleteFile. Entry. Path: /home/runner/work/liberty-tools-intellij/liberty-tools-intellij/liberty-tools-intellij/src/test/resources/projects/maven/singleModMavenMP/target/site/surefire-report.html
    INFO: 2024-08-28T16:22:00.680762032: deleteFile. Exit. Deleted: true

The files are deleted or they were not there. The error message is

java.lang.IllegalStateException: Timed out waiting for test report: /home/runner/work/liberty-tools-intellij/liberty-tools-intellij/liberty-tools-intellij/src/test/resources/projects/maven/singleModMavenMP/target/site/failsafe-report.html file to be created.
        at io.openliberty.tools.intellij.it.TestUtils.validateTestReportExists(TestUtils.java:396)

and the file name matches what was deleted earlier. The second file (surefire-report.html) is not sought and the test case waits 60s after the file check before stopping the server.

turkeylurkey commented 2 months ago

When it says Failed to run the Liberty: View Gradle config action... it also says Retrying... and this works so it is not a failure.

turkeylurkey commented 2 months ago

We have been using target/site as the directory for Junit test results but in this current run the directories in target do not include site.

image

There are some other directories that look like they could hold the test results but why has there been a change?

anusreelakshmi934 commented 2 months ago

In a linux test the log shows what files are being examined:

MavenSingleModMPProjectTest > testRunTestsActionUsingSearch() STANDARD_OUT
    INFO: 2024-08-28T16:22:00.675403834: MavenSingleModMPProjectTest.testRunTestsActionUsingSearch(). Entry
    INFO: 2024-08-28T16:22:00.680408280: deleteFile. Entry. Path: /home/runner/work/liberty-tools-intellij/liberty-tools-intellij/liberty-tools-intellij/src/test/resources/projects/maven/singleModMavenMP/target/site/failsafe-report.html
    INFO: 2024-08-28T16:22:00.680448806: deleteFile. Exit. Deleted: true
    INFO: 2024-08-28T16:22:00.680725534: deleteFile. Entry. Path: /home/runner/work/liberty-tools-intellij/liberty-tools-intellij/liberty-tools-intellij/src/test/resources/projects/maven/singleModMavenMP/target/site/surefire-report.html
    INFO: 2024-08-28T16:22:00.680762032: deleteFile. Exit. Deleted: true

The files are deleted or they were not there. The error message is

java.lang.IllegalStateException: Timed out waiting for test report: /home/runner/work/liberty-tools-intellij/liberty-tools-intellij/liberty-tools-intellij/src/test/resources/projects/maven/singleModMavenMP/target/site/failsafe-report.html file to be created.
        at io.openliberty.tools.intellij.it.TestUtils.validateTestReportExists(TestUtils.java:396)

and the file name matches what was deleted earlier. The second file (surefire-report.html) is not sought and the test case waits 60s after the file check before stopping the server.

At the beginning of the test we first Delete any existing test report files if already present. So it deletes the failsafe-report.html file in target/site. Then the test begins and run tests action occurs. After the run tests. the new failsafe-report.html will be created in the target/site. It seems like the file is not being created again.

anusreelakshmi934 commented 2 months ago

We have been using target/site as the directory for Junit test results but in this current run the directories in target do not include site. image There are some other directories that look like they could hold the test results but why has there been a change?

target/site will only be created after we run the tests. And locally it is being created correctly. Also we will not be able to see target/site generated if the target is directory is expanded already. We must minimise and again expand the directory.

turkeylurkey commented 2 months ago

I do not recall seeing this message in devmode before:

[INFO] Rendering content with org.apache.maven.skins:maven-default-skin:jar:1.3 skin.

I presume the skin is the art style of the html in the site directory. So if there is a new message then somehow we are getting new code.

For context:

image
aparnamichael commented 2 months ago

Maven surefire plugin and maven failsafe plugin updated on Aug 27th. We suspect this release is causing problem with html report.

https://mvnrepository.com/artifact/org.apache.maven.plugins/maven-failsafe-plugin https://mvnrepository.com/artifact/org.apache.maven.plugins/maven-surefire-plugin

turkeylurkey commented 2 months ago

This is more of a question than a finding or an answer. Here we start running unit tests and downloading dependencies.

image

After downloading 12 jars the tests run but none of the downloads are failsafe jar

image

Why is it downloading surefire but not failsafe?

turkeylurkey commented 2 months ago

I tried to specify surefire 3.5.0 in pom.xml in the test case but I could not reproduce the issue. I deleted the .m2/repository locally and forced everything to be downloaded and I did reproduce the issue.

To make things simpler for you try just deleting the directory .m2/repository/org/apache/maven to see if you can reproduce the issue locally.

TrevCraw commented 2 months ago

We should test the Run Tests and View Test Report actions with our current release (24.0.6) to see if we are broken in production.

turkeylurkey commented 2 months ago

Same problem exists with 24.0.6. I used my machine that exhibited the problem using our dev code and I ran the same test using 24.0.6.

image

No html files in site.

TrevCraw commented 2 months ago

I think we want to try to understand a few things:

  1. What is dev mode's connection with the surefire and failsafe plugins?
  2. Why is the latest version of these plugins being downloaded? And do we always want to use the latest version?
  3. What changes in the latest plugin version (3.5.0) of surefire or failsafe is causing Liberty Tools / dev mode to break?
turkeylurkey commented 2 months ago

In dev mode we run this code for unit tests:

  runTestMojo("org.apache.maven.plugins", "maven-surefire-plugin", "test", currentProject);
  runTestMojo("org.apache.maven.plugins", "maven-surefire-report-plugin", "report-only", currentProject);

which means mvn org.apache.maven.plugins:maven-surefire-plugin:test etc. For integration tests:

  runTestMojo("org.apache.maven.plugins", "maven-failsafe-plugin", "integration-test", currentProject);
  runTestMojo("org.apache.maven.plugins", "maven-surefire-report-plugin", "failsafe-report-only", currentProject);
  runTestMojo("org.apache.maven.plugins", "maven-failsafe-plugin", "verify", currentProject);

We don't specify version numbers so it probably just uses 'RELEASE' to use the latest. https://github.com/OpenLiberty/ci.maven/blob/d8c90d8f2d8b30b19c68bab2a5601d91d4f488fb/liberty-maven-plugin/src/main/java/io/openliberty/tools/maven/server/DevMojo.java#L1266

turkeylurkey commented 2 months ago

I tried running this command in the terminal to force v3.4 to be downloaded before starting the server and running tests in the Liberty Tool Window.

./mvnw org.apache.maven.plugins:maven-surefire-report-plugin:3.4.0:report-only 

I verified that v3.4 was downloaded into the local repository but when I started the server and used Run Tests it still downloaded v3.5.0 and LTI did not produce the correct html files.

Edit: Note that when I specified 3.4 like this 3.4 was downloaded but none of the .pom and .xml files were downloaded. Perhaps because plugin dependency analysis was not required when the version number is specified.

turkeylurkey commented 2 months ago

There are html files: image

anusreelakshmi934 commented 2 months ago

In the latest version of Maven Surefire (3.5.0), significant changes were introduced, as detailed in SUREFIRE-2161. Among the changes, the names of the generated reports have been updated—failsafe-report.html is now failsafe.html, and surefire-report.html is now surefire.html. Additionally, the file paths for these reports have been altered. Previously, the HTML report was generated at $\{basedir\}/target/site/surefire-report.html. It is now located at $\{basedir\}/target/reports/surefire.html.

Due to this change, our test, which checks for the report in the target/site directory, is now failing. Attaching the PR link of the changes in maven surefire- https://github.com/apache/maven-surefire/pull/629/files?diff=unified&w=0

anusreelakshmi934 commented 2 months ago

I ran a build by changing the path and name of the report file. The build was success - https://github.com/anusreelakshmi934/liberty-tools-intellij/actions/runs/10628161834/job/29462481922.

aparnamichael commented 2 months ago

@TrevCraw @turkeylurkey

In our LTI code, we are hard coding this path for the actions "view integration test report" and "view unit test report". The change of directory will also affect the functionality of both these liberty actions.

  // get path to project folder
        final VirtualFile parentFile = buildFile.getParent();
        File failsafeReportFile = Paths.get(parentFile.getPath(), "target", "site", "failsafe-report.html").normalize().toAbsolutePath().toFile();

After clearing the m2 in our local and then if we click on "view integration test report" and "view unit test report", it will throw an error "Test report does not exist", as we have hardcoded the above path in our code but the actual test report file is inside "/target/reports". If we change the hardcoded path to "/target/reports" then existing users who use old plugin version, will get the "Test report does not exist", because their report will still generate inside "site" folder.

turkeylurkey commented 2 months ago

In the case it was working there were 4 or 5 different versions of maven-surefire-report-plugin in the local repository. Also there were some .pom and .xml files as well. The lowest version number was 3.2.5. I suspect these other files are necessary for dev mode to have the plugins in its internal management data structure. When dev mode finds this plugin locally it does not need to download the latest release. Perhaps we could use this to work around the issue. I.e. run some command that causes these files to be loaded in the local repository before we run dev mode. This would be a work around until we fix dev mode itself to specify just one version of maven-surefire-report-plugin so that we can depend upon its behaviour.

TrevCraw commented 2 months ago

I am not sure if the fix for this will be specifying one version of the maven-surefire-report-plugin for dev mode to use. I don't know if dev mode should be limited to a specific version or continue to pull the latest. I feel like this issue needs wider discussion amongst the guild (cc @cherylking @scottkurz).

From a Liberty Tools perspective, even if dev mode specifies a version to download, a user could install any version of these plugins that they would like. Therefore, I think we need to be able to handle the behaviour pre 3.5.0 and post 3.5.0. This issue will likely also affect Liberty Tools for Eclipse and Liberty Tools for VS Code. Below are a couple potential solutions. I am open to others.

  1. Liberty Tools looks for the version of maven-surefire-report-plugin installed (and the corresponding failsafe plugin) and then looks for the test report based on the version.
  2. Liberty Tools always looks in both potential test report locations and displays the test report that is found. (I guess in this case we would need to determine a priority location in the unlikely event a test report is in both locations)
scottkurz commented 2 months ago

Agree this is really about the change in maven-surefire-report-plugin version than changes in the surefire/failsafe plugin itself. From liberty:dev, IIUC, we are calling maven-surefire-report-plugin without specifying a version, so we're subjecting ourselves to different behavior depending on what the user happens to have installed.

This does seem to call out for a design discussion. Should we change dev mode or the Liberty Tools IDE tools? If we're changing Liberty Tools should we just add one more default location or should we go to the step of actually reading the user's config (which could be set to a whole other location). I'm not sure I see us going in a whole other step of using an IDE-specific config of our own... but I'm not sure that's out of the question either.

TrevCraw commented 2 months ago

Since the next LTI release is coming soon, we are going to move forward with a fix for LTI. We can always make changes if necessary based on the results of the design discussion. For now, we are going to check both test report locations, looking at the new location first.


https://github.com/OpenLiberty/liberty-tools-intellij/issues/939 opened to track adding fix to LTI

turkeylurkey commented 2 months ago

Idea #1 wont necessarily work because there are typically multiple versions of a plugin in the local repository. image I didn't even do much to get these 5 versions. Also the test case could hard code a version of a plugin but that may not affect the choice dev mode makes.