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

Enable the pde-build tests #998

Merged
merged 1 commit into from
Dec 16, 2023

Conversation

laeubi
Copy link
Contributor

@laeubi laeubi commented Dec 10, 2023

Currently pde test are not executed what leads to regressions detected too late. This enables the tests based on a profile only executed for verification builds.

Please note I have reorganized things a bit to have two explicit executions for tests while the default is suppressed. I also explicitly enabled P2 tests as for me it seems they are all disabled by default.

Locally the ScriptGenerationTests.testBug238177:873 expected:<4> but was:<5> fails, I'll submit this anyways here so maybe we can see if it also fails at Jenkins/Github.

This also currently disables all tests that use a delta-pack I plan to look into this in a separate step if this one is submitted.

Copy link

github-actions bot commented Dec 10, 2023

Test Results

     274 files  +    4       274 suites  +4   55m 22s ⏱️ + 6m 37s
  3 495 tests +166    3 459 ✔️ +160    36 💤 +  6  0 ±0 
10 616 runs  +332  10 514 ✔️ +320  102 💤 +12  0 ±0 

Results for commit d2a16fb. ± Comparison against base commit 6f744c5.

♻️ This comment has been updated with latest results.

@laeubi
Copy link
Contributor Author

laeubi commented Dec 11, 2023

So we have now 3 P2 test failures, @iloveeclipse can you confirm that these are actually run in the ibuild or is my analysis correct that they are actually disabled there?

If they are disabled I would propose to disable them here as well, or maybe even delete them WDYT @akurtakov ?

@iloveeclipse
Copy link
Member

Jenkins:

Test Result (3 failures )
org.eclipse.pde.build.internal.tests.p2.P2Tests.testBug265564
org.eclipse.pde.build.internal.tests.p2.PublishingTests.testBug283060
org.eclipse.pde.build.internal.tests.p2.PublishingTests.testPublish_Packaging_1

SDK Linux build:

https://download.eclipse.org/eclipse/downloads/drops4/I20231211-0900/testresults/html/org.eclipse.pde.build.tests_ep431I-unit-cen64-gtk3-java17_linux.gtk.x86_64_17.html#P2TestSuite

I see tests executed on SDK build with no fails.

@iloveeclipse
Copy link
Member

Two from tests fail because some features couldn't be found. I assume the tycho need to include them into the target platform for tests. Third fail is a NPE, haven't checked why.

Copy link
Member

@HannesWell HannesWell left a comment

Choose a reason for hiding this comment

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

Thank @laeubi for bringing this up again.

So the profile makes the difference to #980 to prevent the cycle.

Another point is that it looked like the activated tests only use what have been build with the I-builds (see #984) and if I understand the config right, this is no surprise since the test-runtime is p2 provisioned?

build/org.eclipse.pde.build.tests/pom.xml Show resolved Hide resolved
build/org.eclipse.pde.build.tests/pom.xml Outdated Show resolved Hide resolved
build/org.eclipse.pde.build.tests/META-INF/p2.inf Outdated Show resolved Hide resolved
@laeubi
Copy link
Contributor Author

laeubi commented Dec 12, 2023

Another point is that it looked like the activated tests only use what have been build with the I-builds (see #984) and if I understand the config right, this is no surprise since the test-runtime is p2 provisioned?

This is sadly right (but not the conclusion), this is not because we use p2 provisioned this only means that P2 is used to build the test runtime (what for example also executes touch points, creates profiles and so on) while otherwise Tycho do the copy itself.

The problem is that currently I use the SDK feature, but as it is not build in this reactor, it contains strict version ranges and therefore the currently build versions are not selected. So I need to somehow convince the p2 director to update the pde(build) features to the build versions ones.

@HannesWell
Copy link
Member

he problem is that currently I use the SDK feature, but as it is not build in this reactor, it contains strict version ranges and therefore the currently build versions are not selected. So I need to somehow convince the p2 director to update the pde(build) features to the build versions ones.

Understand.
Looking at the sdk.product and the sdk feature I think the relevant parts boil down to the platform, jdt and pde feature:
grafik

I wonder if we can just list those three explicitly? Which would always pull in the pde feature just build.
Maybe you also need the feature org.eclipse.equinox.executable for the launcher?

@laeubi
Copy link
Contributor Author

laeubi commented Dec 12, 2023

@HannesWell fell free to checkout the PR and make some test locally, the good thing is its quite easy to execute, but p2 installed runtime has some caveats, e.g. it complains that the launcher lib is missing, so one needs to include that as well, also the executable has to be part of the (not yet) supported deltapack provisioning and so on ... but I'm currently working on make this allow to use the new director mojo and even tycho-surefire to simply use a (externally) provisioned install.

@laeubi
Copy link
Contributor Author

laeubi commented Dec 13, 2023

I see tests executed on SDK build with no fails.

Alright I was confused by this obfuscated statement here:

assumeTrue(Boolean.valueOf(System.getProperty("pde.build.includeP2", "true")).booleanValue());

even though I don't understand why we need the property at all if in the test.xml the suites are explicitly mentioned, so they should never gets executed together anyways and even then I don't see the value to have disable this ... anyways it seems both are wanted in one way or the other.

@laeubi laeubi force-pushed the enable_pde_build_test_take_3 branch from d593d72 to a1f1c5a Compare December 16, 2023 07:26
@laeubi
Copy link
Contributor Author

laeubi commented Dec 16, 2023

Another point is that it looked like the activated tests only use what have been build with the I-builds (see #984)

This is now fixed, I'm using the new director-mojo now to first created a provisioned sdk install, the test bundle is the installed on top of it, according to my local testing this then installs the version from the build (if it is newer).

@laeubi
Copy link
Contributor Author

laeubi commented Dec 16, 2023

@jukzi I noticed that CodeQL seem to execute the test, is this really required for code analysis or can we have it -DskipTests (compile but not execute tests)

@laeubi laeubi force-pushed the enable_pde_build_test_take_3 branch from a1f1c5a to d078fb0 Compare December 16, 2023 07:59
@jukzi
Copy link
Contributor

jukzi commented Dec 16, 2023

@jukzi I noticed that CodeQL seem to execute the test, is this really required for code analysis or can we have it -DskipTests (compile but not execute tests)

isn't taht already done in https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/blame/5fd8db2bf030a053655563e856801255323b0db8/.github/workflows/codeQLworkflow.yml#L86?

@laeubi laeubi force-pushed the enable_pde_build_test_take_3 branch from d078fb0 to 965c4a2 Compare December 16, 2023 08:36
@laeubi
Copy link
Contributor Author

laeubi commented Dec 16, 2023

@jukzi I noticed that CodeQL seem to execute the test, is this really required for code analysis or can we have it -DskipTests (compile but not execute tests)

isn't taht already done in https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/blame/5fd8db2bf030a053655563e856801255323b0db8/.github/workflows/codeQLworkflow.yml#L86?

Yes sorry for confusion, I think the problem is that I explicitly need to enable it need to think how to archive that...

@laeubi laeubi force-pushed the enable_pde_build_test_take_3 branch from 965c4a2 to 5dadafc Compare December 16, 2023 08:59
@laeubi
Copy link
Contributor Author

laeubi commented Dec 16, 2023

I was now able to replicate the deltapack creation as well, there is one test left that probably needs to be enhanced so it does not run into NPE but explains what it was missing.

Currently pde test are not executed what leads to regressions detected
too late.

This enables the tests based on a profile only executed for verification
builds.
@laeubi laeubi force-pushed the enable_pde_build_test_take_3 branch from 5dadafc to d2a16fb Compare December 16, 2023 09:32
@laeubi
Copy link
Contributor Author

laeubi commented Dec 16, 2023

I think the failing test wants that the profile is actually named SDKProfile I adjusted the build now.

@laeubi
Copy link
Contributor Author

laeubi commented Dec 16, 2023

Only usual maven warnings because of http.service

@laeubi laeubi merged commit 7f37e02 into eclipse-pde:master Dec 16, 2023
13 of 17 checks passed
@HannesWell
Copy link
Member

It's great to have this again. Thank @laeubi for this work.

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

Successfully merging this pull request may close these issues.

4 participants