Save screenshots to Maven build output directory#3819
Conversation
basilevs
force-pushed
the
surefire_screenshot
branch
from
0b25779 to
c5aabc8
Compare
|
@merks @HeikoKlare could you suggest a better heuristic or another way to work with CI screenshots in SWT? |
|
I have no knowledge in this area. Maybe @HannesWell has a good idea. |
|
It sounds reasonable to have a properly defined place for storing the screenshots so that they can be archived during GHA runs. I remember that I did some tweaks when analyzing an issue only occurring in GHA runs for which I required screenshots. So having a general solution for this would be great. Thank you for putting effort into this. I wonder if we couldn't just define a proper value for the existing system property Regarind the proposed change: Unless the Eclipse platform is present and the system property for the output folder is specified, the results will now always be written into the surefire target repository (if existing). Is that what we want to happen? For example, when executing the tests locally in the IDE, do we want to have the screenshots placed in a Maven-build-specific folder? In case I have execute a local Maven build at some earlier point in time, that folder will be present and used. Maybe @akurtakov can also help here, as he guarded the screenshot output folder extraction with the Platform being present recently: |
Is not it an application argument and not a system property? |
You're right. Then we maybe want to check for a GHA environment and then define a dedicated folder that is also used for artifact upload? |
basilevs
force-pushed
the
surefire_screenshot
branch
from
c5aabc8 to
c9bbda8
Compare
The decision should be based on whether Maven is in use, not on which environment it is running on. As some projects reuse aggregator's workflow, and most use it's parent POM, the maintenance effort per-project should be minimal. |
basilevs
changed the title
HannesWell
left a comment
HannesWell
left a comment
There was a problem hiding this comment.
Generally I think this is fine.
Do you know if the screenshots are also saved for I-build tests and if some according adaption is needed then? I'm not aware of it, but might have missed it.
| <printApiMessages>false</printApiMessages> | ||
| <failOnJavadocErrors>false</failOnJavadocErrors> | ||
| <compileLogDir>${project.build.directory}/compilelogs</compileLogDir> | ||
| <eclipse.screenshot_directory>${project.build.directory}/screenshots</eclipse.screenshot_directory> |
There was a problem hiding this comment.
Wouldn't the property be better named surefire.screenshot_directory (and be placed next to the other surefire related properties)?
| <eclipse.screenshot_directory>${project.build.directory}/screenshots</eclipse.screenshot_directory> | |
| <surefire.screenshot_directory>${project.build.directory}/screenshots</surefire.screenshot_directory> |
There was a problem hiding this comment.
The JVM system property is naturally usable outside of surefire. For example, it could be configured via launch configuration. This makes naming it surefire specific confusing.
Maven property was named to exactly match the JVM system property to help with identification and search.
I did fix screenshots for platform.ui a few years back. Those relied on |
eclipse-platform/eclipse.platform.swt#3303 As screenshots are effectively a build artifact, save them in build output directory (target/ by default). SWT does not use Platform, so existing heuristics have not worked in GitHub Actions, abandoning screenshots in /tmp without archiving into artifacts. The change breaks existing Gerrit jobs, but as Gerrit is no longer used, this should be fine.
basilevs
force-pushed
the
surefire_screenshot
branch
from
c9bbda8 to
1e5e3cc
Compare
4 participants
Partial fix for eclipse-platform/eclipse.platform.swt#3303.
SWT does not use Platform, so existing heuristics do not work in GitHub Actions.
Once this more sensible screenshot location is set, SWT GHA workflows can be updated to archive screenshots to artifacts. Before the change they have been thrown away and could not be inspected.