Proper Maven command to run tests#3270
Proper Maven command to run tests#3270basilevs wants to merge 2 commits intoeclipse-platform:masterfrom
Conversation
HannesWell
force-pushed
the
copilot/update-agents-md-and-verify-test
branch
from
4d66bc7 to
a68dcd4
Compare
basilevs
force-pushed
the
copilot/update-agents-md-and-verify-test
branch
2 times, most recently
from
47fb010 to
81b3915
Compare
specific tests Prevent the procedure from failing on MacOS and Linux which have no platform specific tests, requiring `surefire.failIfNoSpecifiedTests=false`. Agent-Logs-Url: https://github.com/basilevs/eclipse.platform.swt/sessions/b6e7df41-c662-4223-8f74-48bace109b78
basilevs
force-pushed
the
copilot/update-agents-md-and-verify-test
branch
from
81b3915 to
38927e6
Compare
|
@HeikoKlare please review |
HeikoKlare
left a comment
HeikoKlare
left a comment
There was a problem hiding this comment.
Thank you for the proposal. In general, the change looks fine to me. I've tested if an agent (Copilot CLI with Sonnet 4.6) uses a proper command after applying the proposed change. I used the following prompt with and without this change:
The tests in @tests\org.eclipse.swt.tests.win32\JUnit Tests\org\eclipse\swt\tests\win32\Test_org_eclipse_swt_events_KeyEvent.java are failing when I execute them in the IDE. Can you please confirm that the tests are failing by executing it via Maven?
Without this change, it uses mvn test -Dtest=Test_org_eclipse_swt_events_KeyEvent -pl :org.eclipse.swt.tests.win32 which does not work because of the wrong Maven goal and the missing -am for building dependencies. With this change, it uses mvn verify -Dtest=Test_org_eclipse_swt_events_KeyEvent -pl :org.eclipse.swt.tests.win32 - am and thus works fine (except for the missing -DskipNativeTests=false, see my additional comment).
|
|
||
| # Run specific test class without prior `mvn install` (uses -am to build dependencies inline). | ||
| mvn verify -pl :THE_BUNDLE_WITH_THE_ACTUAL_TEST -am -Dtest=ClassName | ||
| ``` |
There was a problem hiding this comment.
We might add a sentence about OS-specific tests here. In case the test is OS-specific (fully qualified name contains "win32", "cocoa" or "gtk"), native tests must be enabled via -DskipNativeTests=false
There was a problem hiding this comment.
Why is this property needed? Why can't native tests always run?
There was a problem hiding this comment.
I have to admit that I don't know why it is currently configured like that. Maybe it's related to the Jenkins build, as that one does not execute any of the native tests at all. Without setting this properly, tests inside the OS-specific test bundles will not be executed. E.g., the command I mentioned in the review comment mvn verify -Dtest=Test_org_eclipse_swt_events_KeyEvent -pl :org.eclipse.swt.tests.win32 - am will not execute any test without -DskipNativeTests=false.
| <buildid>${buildId}</buildid> | ||
| <maven.compiler.release>21</maven.compiler.release> | ||
| <swtMainProject>${project.basedir}/../../bundles/org.eclipse.swt</swtMainProject> | ||
| <surefire.failIfNoSpecifiedTests>false</surefire.failIfNoSpecifiedTests> |
There was a problem hiding this comment.
Why is this change required?
There was a problem hiding this comment.
It prevents mvn verify -am from failing on MacOS and Linux which have no platform specific tests. There are tests for Windows, so we do not want to remove surefire from platform-specific fragments. This will allow to both run tests on Windows and not to fail on other platforms.
I guess this is why you use -DskipNativeTests. My approach seems cleaner, as tests are only skipped if they are absent for that platform instead of either skipping them altogether or breaking some platforms.
There was a problem hiding this comment.
I see. It is because the native fragments use surefire and when specifying a concrete test inside a different test bundle for execution, there will be no matching tests in the fragments to be executed, so the build fails. But this is independent from -DskipNativeTests (which is relevant for the OS-specific test bundles and not the unit tests inside the fragments) and not specific to any platform (as there are tests inside the native fragments executed on every platform).
In my opinion, setting failIfNoSpecifiedTests to false for the fragments is not correct, as there actually are tests and we want to be notified if we break anything in the configuration that results in non-execution of those tests. I don't have a perfect solution for this. Maybe just adding -Dsurefire.failIfNoSpecifiedTests=false to the selective test execution would be the simplest solution?
mvn testcommand does not work in Tycho Surefire context.Added a proper command.
Do not fail Linux and MacOS testing because of absence of platform-specific tests.
Fixes #3261.