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

[SUREFIRE-1975] JDK18 - The Security Manager is deprecated and will be removed in a future release #422

Merged
merged 1 commit into from Jan 21, 2022

Conversation

Tibor17
Copy link
Contributor

@Tibor17 Tibor17 commented Jan 1, 2022

Following this checklist to help us incorporate your
contribution quickly and easily:

  • Make sure there is a JIRA issue filed
    for the change (usually before you start working on it). Trivial changes like typos do not
    require a JIRA issue. Your pull request should address just this issue, without
    pulling in other changes.
  • Each commit in the pull request should have a meaningful subject line and body.
  • Format the pull request title like [SUREFIRE-XXX] - Fixes bug in ApproximateQuantiles,
    where you replace SUREFIRE-XXX with the appropriate JIRA issue. Best practice
    is to use the JIRA issue title in the pull request title and in the first line of the
    commit message.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Run mvn clean install to make sure basic checks pass. A more thorough check will
    be performed on your pull request automatically.
  • You have run the integration tests successfully (mvn -Prun-its clean install).

If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.

To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.

import org.apache.maven.surefire.common.junit3.JUnit3Reflector;
import org.apache.maven.surefire.api.report.ReportEntry;
import org.apache.maven.surefire.api.report.RunListener;
import org.apache.maven.surefire.api.report.RunMode;
import org.apache.maven.surefire.api.report.TestSetReportEntry;
import org.apache.maven.surefire.shared.lang3.JavaVersion;
Copy link
Member

Choose a reason for hiding this comment

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

In test classes we can use direct comons - not shaded

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@slawekjaranowski
It has certain purpose.

fail();
}

Object sm = invokeGetter( System.class, null, "getSecurityManager" );
Copy link
Member

Choose a reason for hiding this comment

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

System.getSecurityManager() is public ... why by reflections

Comment on lines 50 to 57
public static boolean isSecurityManagerSupported()
{
ClassLoader classLoader = ObjectUtils.class.getClassLoader();
Class<?> smClass = tryLoadClass( classLoader, "java.lang.SecurityManager" );
return smClass != null && !smClass.isAnnotationPresent( Deprecated.class );
}
Copy link
Member

Choose a reason for hiding this comment

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

Is checking java version >= 17 not enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was my previous solution but the class appears in module surefire-booter. This is surefire-api, so we cannot inherit it and cannot duplicate the code.

Copy link
Member

Choose a reason for hiding this comment

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

what about Java Version from commons lang3 is is accessible by org.apache.maven.surefire.shared.lang3.JavaVersion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@slawekjaranowski
Now JavaVersion is in use!

timeout-minutes: 120
steps:
- name: Verify
uses: apache/maven-gh-actions-shared/.github/workflows/maven-verify.yml@v1
Copy link
Member

Choose a reason for hiding this comment

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

it is shared workflow ... can not be uses as step

Copy link
Member

Choose a reason for hiding this comment

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

We can extend this workflow with artifacts upload support

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@slawekjaranowski
This change does not exit in this branch any longer.

@Tibor17
Copy link
Contributor Author

Tibor17 commented Jan 1, 2022

This CI script will publish archived artifacts for further analysis. We need to find out why the JVM exists after one hour of execution.

Comment on lines 29 to 63

strategy:
matrix:
os: [ubuntu-latest, windows-latest, macOS-latest]
java: [8, 11, 17]
jdk: [temurin]
fail-fast: false

runs-on: ${{ matrix.os }}
timeout-minutes: 120

steps:
- name: Checkout
uses: actions/checkout@v1

- name: Set up JDK ${{ matrix.java }}
uses: actions/setup-java@v2.4.0
with:
distribution: ${{ matrix.jdk }}
java-version: ${{ matrix.java }}
cache: 'maven'

- name: Build with Maven
run: mvn clean install -e -B -V -nsu --no-transfer-progress -P run-its -Dfailsafe-integration-test-port=8083

- name: Upload artifact surefire-its
uses: actions/upload-artifact@v2-preview
if: failure()
with:
name: ${{ matrix.os }}-surefire-its
path: |
surefire-its/target/*/log.txt
surefire-its/target/**/surefire-reports/*
surefire-its/target/**/failsafe-reports/*
!surefire-its/target/failsafe-reports
Copy link
Member

Choose a reason for hiding this comment

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

Don't do it in this way
it is step backward

Copy link
Contributor Author

@Tibor17 Tibor17 Jan 1, 2022

Choose a reason for hiding this comment

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

The commit message is investigating. The commit does not mean anything for this Jira issue. We are discussing the timeout in the Slack/INFRA channel and I need to get the archive of surefire-its/target with all the dump files and logs. This was happening one year ago and I came over this problem by splitting the test set in two. Currently I am waiting for the build to complete and I hope the archive files would appear in the attachment. Then I want to go inside and analyse the logs and test content.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@slawekjaranowski
The build runs every configuration twice, means push and pull_request. This is the waste and I also could not avoid it in my scripts. Do you know how to do it? This would save the h/w resources in favor of others.

Copy link
Member

Choose a reason for hiding this comment

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

That is because we have two events:

  • push to branches
  • pull_request

I try to discover how prevent it

Copy link
Member

Choose a reason for hiding this comment

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

by the way attached artifacts has size > 1GB ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@slawekjaranowski
This change does not exit in this branch any longer.

@Tibor17
Copy link
Contributor Author

Tibor17 commented Jan 1, 2022

This investigation has helped me.

It looks like the GH build runs out of disk space.
I made a ZIP of the largest sub-module and its target folder occupies 1.2GB and I deleted binary files. So the real consumption of the disk space must be much bigger.
The problem is that the ZIP does not contain the logs of the last 40 integration test however they were executed but their files are no more on the disk! Still we are somewhere in the middle of the entire test set.
How can we prove my hypothesis other way that the particular build runs out of disk space?

It looks like one build crashed but the other did not.

@Tibor17
Copy link
Contributor Author

Tibor17 commented Jan 1, 2022

@slawekjaranowski
One of our colleagues told me that the Github CI reserves 14GB of disk space. Maybe the problem is that we execute 20 parallel builds. We should try to avoid the duplicates (push, pull_request) which makes 1.4GB per each run configuration. I would propose to additionally overriding the Java version with min and max, means 8 and 17 only, if 10 parallel builds still does not help.

@Tibor17 Tibor17 merged commit aeb6a32 into master Jan 21, 2022
@Tibor17 Tibor17 deleted the SUREFIRE-1975 branch January 21, 2022 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants