Fine-tune testing on Windows

[sormuras]

All Windows issues down

• Skip all bots tests on Windows

[...] and since the bots are not really required to run on Windows any ways, it would also be fine to disable all the bots tests from running on Windows, to avoid any further compatibility problems.

• Convert relative folder path to be URI-friendly
  On Windows Path::toString generates back slashes as path element separators. This yields invalid URI paths: like "%s\webrev.%s".

Progress

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

Approvers

• Erik Helin (ehelin - Reviewer)
• Jorn Vernee (jvernee - Committer)

[bridgekeeper]

👋 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).

[mlbridge]

Webrevs

• 01: Full - Incremental (20b2683)
• 00: Full (91ea9193e5a95432068ecdf1ec7cc13c7e9ddbd7)

[JornVernee]

Thanks for looking into this! With your patch all the mlbridge tests are passing for me. I'm not seeing the same failures as you with the mlbridge tests...

I'm not a Skara reviewer, so I can't Approve™ this, but the patch looks good to me 👍

---

FWIW, I also got some other failures from some of the bot tests, particularly IntegrateTests::autorebase, MergeTests::branchMergeRebase and SponsorTests::autoRebase in the bots.pr module. When looking into it this seemed to be caused by file paths exceeding the maximum path length on Windows. Git rejects them with a specific error message, and HG just throws a file not found error.

Besides that some tests in the bots.submit module are failing as well, but I haven't determined the exact reason yet.

I had a chat with Erik about this, and since the bots are not really required to run on Windows any ways, it would also be fine to disable all the bots tests from running on Windows, to avoid any further compatibility problems. Or, just aggressively disable any that are failing.

[sormuras]

[...] it would also be fine to disable all the bots tests from running on Windows [...]

As long as some CI service is running those tests on Linux/Mac, fine with me.

I'll extend this PR with to disable all bots tests on Windows.

[edvbld]

This looks good, but @sormuras would you mind updating the title of the PR? The title of the PR will be used as the title for the resulting commit, and I think we would want something a bit more descriptive than Windows again... 😄

[sormuras]

Sure, Erik.

But I'll update the PR once more to include comments from above:

• Disable all bots tests on Windows
• Revert not needed optimization -- Gradle does just fine, only IDEA executes disabled tests.

Due to @BeforeAll void setup() (and teardown()) being called anyway, a second gatekeeper in form an assumption is added to this method as well. This improves the execution duration time of all disabled tests.

[edvbld]

Ok, sounds good!

[sormuras]

Sneak-peek: https://gradle.com/s/bzobk5dbt7zga

[sormuras]

Instead of annotating each test class in every bots project with @DisabledOnOs(WINDOWS), I implemented an ExecutionCondition and enabled auto-detection of provided Jupiter extension. That implementation is named DisableAllBotsTestsOnWindows and resides in the org.openjdk.skara.test module. It checks the current test classes' package name. If that package name starts with "org.openjdk.skara.bots." and the current operating system is Windows, the execution of all tests within the test class are disabled.

Too much magic?

If only @ExtendWith could be attached to packages or modules... junit-team/junit5#1986 ;-)

[edvbld]

Looks good to me!

[openjdk]

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

Fine-tune testing on Windows

Reviewed-by: ehelin, jvernee

• 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 11 commits pushed to the master branch:

• 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

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, @JornVernee) 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).

[sormuras]

/integrate

[openjdk]

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

[edvbld]

/sponsor

[openjdk]

@edvbld @sormuras The following commits have been pushed to master since your change was applied:

• 25a8f8f
• 4fb0315
• 7a927a0
• bcd21d8
• 984bb12
• c7ffefa
• 03ea4f9
• b9277c9
• 31549d0
• 9ca8782
• f3a450e

Your commit was automatically rebased without conflicts.
Pushed as commit 7ea3269.