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

Move platform jobs to 11 #6617

Merged
merged 4 commits into from Oct 27, 2023
Merged

Move platform jobs to 11 #6617

merged 4 commits into from Oct 27, 2023

Conversation

mbien
Copy link
Member

@mbien mbien commented Oct 24, 2023

platform jobs:

  • test everything on JDK 11 which can run on JDK 11
  • fix some of the tests which are "easily" fixable
    • openide.filesystems
    • options.api
    • openide.awt
    • core.multiview
    • platform/favorites
  • leave the rest in a JDK 8 section of the platform job for now (makes it easy to see what still needs work)
  • remove relict from travis times: can't find any references to -Dvanilla.javac.exists=true

misc jobs:

meta issue #4904
rebase after #6579 is merged (done)

@mbien mbien added Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) CI Continuous Integration Platform [ci] enable platform tests (platform/*) labels Oct 24, 2023
@mbien mbien added this to the NB21 milestone Oct 24, 2023
@mbien mbien removed the Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) label Oct 24, 2023
@mbien mbien force-pushed the platform-to-11 branch 2 times, most recently from adfbe00 to 2732d61 Compare October 24, 2023 18:43
@mbien mbien changed the title Move platform job to 11 Move platform jobs to 11 Oct 24, 2023
@mbien
Copy link
Member Author

mbien commented Oct 24, 2023

appears to be specific to zulu JDK 11. Can't reproduce it with other vendors. Could be still a race condition, have to check if the test case is known to fail.

junit.framework.AssertionFailedError: Favorites remain in the same order expected:<[/home/runner@6edb6b40:6867f7fd, /home/runner/work/netbeans/netbeans/platform/favorites/build/test/unit/work/o.n.m.f.a.F/testBasicCycle@cf372ff2:4d1c005e, /opt/hostedtoolcache/Java_Zulu_jdk/11.0.21-9/x64@3350f69a:194aeff8, /home/runner/work/netbeans/netbeans/platform/favorites/build/test/unit/work/o.n.m.f.a.F/testBasicCycle/test.txt@8f6c68d1:8462f31]> but was:<[/home/runner@6edb6b40:6867f7fd, /home/runner/work/netbeans/netbeans/platform/favorites/build/test/unit/work/o.n.m.f.a.F/testBasicCycle@cf372ff2:4d1c005e, /home/runner/work/netbeans/netbeans/platform/favorites/build/test/unit/work/o.n.m.f.a.F/testBasicCycle/test.txt@8f6c68d1:8462f31, /opt/hostedtoolcache/Java_Zulu_jdk/11.0.21-9/x64@3350f69a:194aeff8]>
at org.netbeans.modules.favorites.api.FavoritesTest.testBasicCycle(FavoritesTest.java:90)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at org.netbeans.junit.NbTestCase.access$200(NbTestCase.java:79)
at org.netbeans.junit.NbTestCase$2.doSomething(NbTestCase.java:484)
at org.netbeans.junit.NbTestCase$1Guard.run(NbTestCase.java:405)
at java.base/java.lang.Thread.run(Thread.java:829)

edit: its not caused directly by zulu, it is more due to the fact that z comes at the end of the alphabet and changes the ordering at some point since java home is used as test data.

edit2: I am going to modify the test slightly, this might be a minor bug which only occurs when multiple items are added to the favorites list at the same time (multi select). The code is not documented and it is not obvious to me how the sorting works internally, so I don't want to modify this for little gain / high risk. I left some comments in the code.

@mbien mbien added the Gradle [ci] enable "build tools" tests label Oct 25, 2023
JDK folder name did influence item order, which made the test not
portable.

Minor performance improvement in favorites/Actions, loop can be left
once root is found.
 - test everything on JDK 11 which can run on JDK 11
 - leave the rest in a JDK 8 section of the platform job for
   now (makes it easy to see what still needs work)
 - remove relict from travis times:
   can't find any references to -Dvanilla.javac.exists=true

other jobs:

 - bump java debugger tests from [11, 17] to [11, 21]
 - bump build-tools job from [11, 17] to [11, 21], isolate tests
   which can't run on JDK 21 yet and run them on 17
@mbien mbien marked this pull request as ready for review October 25, 2023 20:08
Comment on lines -152 to -172
/**
* Checks whether the version of javac in use suffers from #6929404.
* If so, calls to {@code LayerBuilder.validateResource(..., true)} will return normally
* even if the resource path does not exist, so tests must be more lenient.
*/
public static boolean searchClasspathBroken() {
// Cannot just check for e.g. SourceVersion.RELEASE_7 because we might be running JDK 6 javac w/ JDK 7 boot CP, and that is in JRE.
// (Anyway libs.javacapi/external/nb-javac-api.jar, in the test's normal boot CP, has this!)
// Filter.class added in 7ae4016c5938, not long after f3323b1c65ee which we rely on for this to work.
// Also cannot just check Class.forName(...) since tools.jar not in CP but ToolProvider loads it specially - not true anymore since JDK 9 ToolProvider does not look for tools.jar.
final String res = "com/sun/tools/javac/util/Filter.class"; //NOI18N
final CodeSource codeSource = ToolProvider.getSystemJavaCompiler().getClass().getProtectionDomain().getCodeSource();
if (codeSource != null) {
//Compiler from URLClassLoader - JDK7, JDK8 javac
return new URLClassLoader(new URL[] {codeSource.getLocation()}).findResource(res) == null;
} else {
//Compiler from Boot, Ext, System ClassLoader - JDK9 javac
return ClassLoader.getSystemClassLoader().getResource(res) == null;
}
}

Copy link
Member Author

Choose a reason for hiding this comment

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

if I understand this correctly, this shouldn't be relevant anymore and can be simply removed

Comment on lines 423 to 430
//Find root
for (DataObject children1 : children) {
FileObject fo = children1.getPrimaryFile();
if ("Favorites/Root.instance".equals(fo.getPath())) {
//NOI18N
root = children1;
for (DataObject child : children) {
FileObject fo = child.getPrimaryFile();
if ("Favorites/Root.instance".equals(fo.getPath())) { //NOI18N
root = child;
break;
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

exits the loop now when root is found as the comment says.

Comment on lines +903 to 915
# TODO fix JDK 21 incompatibilites
- name: java/java.mx.project
continue-on-error: ${{ github.event_name != 'pull_request' }}
run: .github/retry.sh ant $OPTS -f java/java.mx.project test

- name: java/gradle.java
run: .github/retry.sh ant $OPTS -f java/gradle.java test

- name: extide/gradle
run: ant $OPTS -f extide/gradle test

- name: extide/o.apache.tools.ant.module
run: ant $OPTS -f extide/o.apache.tools.ant.module test
Copy link
Member Author

Choose a reason for hiding this comment

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

gradle should fix itself automatically once it is actually compatible with JDK 21

java.mx.project and o.apache.tools.ant.module failed on JDK 21

Comment on lines +1075 to +1083
# TODO fix JDK 11 incompatibilities
- name: platform/lib.uihandler
run: ant $OPTS -f platform/lib.uihandler test

- name: platform/openide.text
run: .github/retry.sh ant $OPTS -f platform/openide.text test

- name: platform/openide.util.ui
run: ant $OPTS -f platform/openide.util.ui test
Copy link
Member Author

Choose a reason for hiding this comment

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

openide.util.ui has some failures due to JDK xml lib updates. One is a whitespace problem since whitespace parsing changed and is parsed into nodes, the other is a formatting problem. The formatting test is probably not easily fixable and might not be worth it to fix. I have a patch for the other failure locally.

Forgot specifics about the other two modules.

@mbien mbien merged commit fb08355 into apache:master Oct 27, 2023
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration DO NOT squash Gradle [ci] enable "build tools" tests Platform [ci] enable platform tests (platform/*)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants