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

Upgrade JUnit 5 to 5.5.1 #66

Closed
wants to merge 2 commits into from

Conversation

@sormuras
Copy link

sormuras commented Aug 22, 2019

Upgrade JUnit 5 to Jupiter 5.5.1 and Platform 1.5.1

  • Set Jupiter version number 5.5.1
  • Introduce Platform Launcher 1.5.1 test runtime dependency in order to force Gradle to load the module from the module-path
  • Add missing "requires 'org.junit.jupiter.params'" directives in modules: jcheck, storage, vcs, and webrev

Solves https://bugs.openjdk.java.net/projects/SKARA/issues/SKARA-69

Progress

  • Change must not contain extraneous whitespace
  • Change must be properly reviewed

Approvers

  • Erik Helin (ehelin - Reviewer)
- Set Jupiter version number 5.5.1
- Introduce Platform Launcher 1.5.1 test runtime dependency
  in order to force Gradle to load the module from the module-path
- Add missing "requires 'org.junit.jupiter.params'" directives
  in modules: jcheck, storage, vcs, and webrev

Solves https://bugs.openjdk.java.net/projects/SKARA/issues/SKARA-69
@bridgekeeper

This comment has been minimized.

Copy link

bridgekeeper bot commented Aug 22, 2019

👋 Welcome back cstein! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request (refresh this page to view it).

@openjdk openjdk bot added build rfr labels Aug 22, 2019
@mlbridge

This comment has been minimized.

Copy link

mlbridge bot commented Aug 22, 2019

Webrevs

testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine:5.5.1'
// Force Gradle to load the JUnit Platform Launcher from the module-path, as
// configured in buildSrc/.../ModulePlugin.java -- see SKARA-69 for details.
testRuntimeOnly 'org.junit.platform:junit-platform-launcher:1.5.1'

This comment has been minimized.

Copy link
@edvbld

edvbld Aug 22, 2019

Member

Hmm, I don't really follow here - how does adding a test runtime dependency on org.junit.platform:junit-platform-launcher:1.5.1 force Gradle to load the JUnit Platform launcher from the module path? Did you forgot to git add a change made to buildSrc/.../ModulePlugin.java?

This comment has been minimized.

Copy link
@sormuras

sormuras Aug 22, 2019

Author

Did you forgot to git add a change...

No. The re-configuration of the Gradle test task is good as-is.

IIRC and locally passing tests support that, Gradle uses JUnit Platform Launcher (and its dependencies) from the project dependencies -- and only injects one (on the class-path) if a project doesn't provide it.

With org.junit.platform:junit-platform-launcher:1.5.1 a) being present on the project's class-path and b) being moved to the module-path by the module plugin, things work out again.

Correct me if I'm wrong here, @marcphilipp, @britter, et al.

This comment has been minimized.

Copy link
@edvbld

edvbld Aug 23, 2019

Member

Ah ok, I get, thanks for the explanation @sormuras! I'm pretty busy today, but I will take the patch for a spin early next week 👍

This comment has been minimized.

Copy link
@marcphilipp

marcphilipp Sep 2, 2019

Gradle always puts the version of the junit-platform-launcher it ships with on the classpath. Adding org.junit.platform:junit-platform-launcher:1.5.1 to the module path seems to hide that version from the classloader which now loads the version on the module path instead. This version is a proper module and may access the org.junit.platform.commons.util package of the junit-platform-commons module.

@sormuras sormuras changed the title Upgrade JUnit 5 Upgrade JUnit 5 to 5.5.1 Aug 27, 2019
@edvbld
edvbld approved these changes Sep 16, 2019
Copy link
Member

edvbld left a comment

@sormuras I finally had a chance to properly review and test this patch. Everything works great, nice job!

@openjdk openjdk bot removed the rfr label Sep 16, 2019
@openjdk

This comment has been minimized.

Copy link

openjdk bot commented Sep 16, 2019

@sormuras This change can now be integrated. The commit message will be:

Upgrade JUnit 5 to 5.5.1

Reviewed-by: ehelin
  • If you would like to add a summary, use the /summary command.
  • To list additional contributors, use the /contributor command.

Since the source branch of this PR was last updated there have been 70 commits pushed to the master branch:

  • 2b9e3d5: Add tags to subjects instead of replacing them
  • 7ecca5b: Add currentBookmark method to TestRepository
  • 3cece6c: 101: Add support for notifying an RFR thread when a PR is integrated
  • 2ae213d: Enable Mercurial support for "pr create"
  • 80d1e6a: Adjust collapsing of multiple emails
  • a15518e: Adjust reply subjects
  • 409464d: Add Mercurial mapping for skara command
  • 3ae6b4e: Use either the contributor mail or a host-specific noreply address
  • 81c6aae: Better Mercurial compatability
  • e54f813: Add markdown filtering
  • 39c5189: 73: Improve email threading, add separate approval email
  • 96b7146: Escape quotes in shell code stored in gitconfig.
  • d14976e: Add support for Mercurial
  • 4a88959: Add tool for importing webrev patches
  • 4cf872c: Fix configuration parse error
  • 7cf1b08: 76: Remove the "ready" label from a PR once it is integrated
  • 418efc9: Allow more compact bot configuration
  • 4c6b8ae: Coalesce hunks that have overlapping context
  • ac12500: Allow reverse mapping of census namespace users
  • 2aa4482: Extend the repository configuration syntax
  • a0e7eda: Topological bot tests needs to read org.junit.jupiter.params
  • a2d8107: 8: Use a file based approach for GitToHgConverter.java
  • bb34b1d: Add merge and topological bots to cli image
  • cbe145f: Make sure Commits are closed when using Commits::stream
  • 8501faf: Add topological merge bot
  • c999004: Batch commit, add and files methods in HgRepository
  • 1f95176: 91: Avoid discarding pending items of different types
  • b46b8e5: jcheck "repository" property in [general] section not read from .jcheck/conf
  • 417c6f5: Avoid running PullRequestBot WorkItems for the same PR concurrently
  • 59c6145: 88: git token store https://github.com does not work
  • 910b2c8: Add merge bot
  • 660d626: Add Repository::abortMerge
  • f4de221: Add optional handler for workitem runtime errors
  • 482ee91: TestHostedRepository::getPullRequests should only return open PRs
  • 7ea3269: Fine-tune testing on Windows
  • 25a8f8f: Add ReadOnlyRepository::contains
  • 4fb0315: Add newline at end of file to MergeTests.java
  • 7a927a0: Add default safe methods for Repository::checkout
  • bcd21d8: TestPullRequest::getBody always returns empty string
  • 984bb12: 53: Decouple review requirement from the jcheck status
  • c7ffefa: Try adapting to malformed mboxes
  • 03ea4f9: 51: Authentication token generation failure
  • b9277c9: Filter out huge files to avoid excessive storage use
  • 31549d0: Allow the notify bot to watch multiple branches
  • 9ca8782: Parse .ssh/config in git-pr if needed
  • f3a450e: Split large webrev pushes
  • 7dbc76d: Special domain for OpenJDK Contributors is openjdk.org
  • 4e470d8: Limit path arguments to git to 64 at a time
  • a4f0ec5: 79: jcheck allows self-reviews when processing pull requests
  • 373607b: Ignore line ending differences in most tests, to compensate for autocrlf
  • e8e2f0e: Make webrev reject positional arguments
  • c730de2: Fix webrev generation when target hunk is empty
  • 3436186: Add ReadOnlyRepository::status
  • 5799042: Add ReadOnlyRepository::dump
  • 20c3f55: Adding project explanation
  • 9c156f5: Expect carriage return to be part of lines when testing on Windows.
  • 1c632fa: Allow output path to be created with arbitrary directory nesting
  • 43d34cc: Avoid using getUrl when creating folder names
  • 16c6536: Add ReadOnlyRepository::files
  • 2a98c8a: Force unix line endings when writing patch files
  • 24f3dde: Improve health check for git repositories
  • c582fb5: Do not catch NoSuchElementException in HgOpenJDKImport
  • 256e498: Make ext.py compatible with hg5 api
  • 6d6a0b6: A repository without refs but with objects is healthy
  • 69d6b6b: Fix 2 failing tests due to URI syntax errors
  • b4b37e8: Fix some issues building on Windows
  • 1b97234: Convert path separators in patch file to unix explicitly
  • 0f1e47d: Parse list of repositories to mirror as a list
  • ac33f2f: Add support for ready markers to the PR bot as well
  • 8467aae: 61: Remove sponsor label if PR is updated after flagged for integration

Since there are no conflicts, your changes will automatically be rebased on top of the above commits when integrating. If you prefer to do this manually, please merge master into your branch first.

As you do not have Committer status in this project, an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@edvbld) but any other Committer may sponsor as well.

  • To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).
@openjdk openjdk bot added the ready label Sep 16, 2019
@sormuras

This comment has been minimized.

Copy link
Author

sormuras commented Sep 16, 2019

Before making this PR integration-ready, shall I update to JUnit 5.5.2 right away? We fixed two minor bugs: https://junit.org/junit5/docs/5.5.2/release-notes/

@edvbld

This comment has been minimized.

Copy link
Member

edvbld commented Sep 16, 2019

Lets do that as a separate PR, I will have to test that one as well. Now that I've finally finished testing and reviewing this one lets integrate it 👍

@sormuras

This comment has been minimized.

Copy link
Author

sormuras commented Sep 16, 2019

/integrate

@openjdk openjdk bot added the sponsor label Sep 16, 2019
@openjdk

This comment has been minimized.

Copy link

openjdk bot commented Sep 16, 2019

@sormuras
Your change (at version 732b8f7) is now ready to be sponsored by a Committer.

@edvbld

This comment has been minimized.

Copy link
Member

edvbld commented Sep 16, 2019

/sponsor

@openjdk openjdk bot closed this Sep 16, 2019
@openjdk openjdk bot added integrated and removed sponsor ready labels Sep 16, 2019
@mlbridge

This comment has been minimized.

Copy link

mlbridge bot commented Sep 16, 2019

Mailing list message from duke duke@openjdk.java.net

Changeset: 3e6357c
Author: Christian Stein
Committer: Erik Helin
Date: 2019-09-16 14:19:34 +0000
URL: https://git.openjdk.java.net/skara/commit/3e6357c7

Upgrade JUnit 5 to 5.5.1

Reviewed-by: ehelin

! README.md
! build.gradle
! jcheck/build.gradle
! storage/build.gradle
! vcs/build.gradle
! webrev/build.gradle

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

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.