Skip to content

Conversation

@jpobst
Copy link
Contributor

@jpobst jpobst commented Jan 23, 2020

This PR is intended to give us greater control over when we run our extended regression test suite and to parallelize TimeZoneInfo tests.

Create RunAllTests variable

Creates a new RunAllTests variable that defaults to false. This variable is used as a condition to trigger running our extended regression test suite.

This variable is set to true in the following ways:

  • The master pipeline sets the value to true so all commits to master run the full suite.
  • The value is set to true for a PR build if:
    • The branch name contains mono-
    • The branch is not a fork
    • This allows our Mono bump branches PRs (which take the form of mono-2019-12) to run the full suite, as there is a much higher chance of breakage for these commits.
  • If you are queuing a new run from the UI, clicking the Variables option will allow you to set it to true for that run. (The variable shows up with the default value so you don't have to remember its name.)

Put extended suites behind RunAllTests variable

The following test suites are behind this new variable:

  • Integrated Regression - MacOS
  • Integrated Regression - Windows
  • TimeZoneInfo - MacOS

The effective changes are:

  • Previously the integrated regression suite also ran for non-fork PR builds.
  • Previously the TZI suite ran on all PR builds.

We have determined that these suites are unlikely to break for normal day-to-day changes, so by default they will now only run on master commits and Mono bump PRs.

As stated above, if you feel your change may be risky you can manually kick off a full test in the Pipelines UI.

Updates the regression test stage introduced in commit 47e00d7 to only
run when the mac_build stage completes successfully, as it depends
on artifacts produced by that stage.

Parallelize TimeZoneInfo tests

The TimeZoneInfo test suite has been split across 3 build nodes to reduce each run below the target of 1 hour. (Previously this step took over 2 hours.)

Testing

Note: Steps not related to this change were canceled or skipped.

@jpobst jpobst force-pushed the parallel-timezoneinfo branch from 84c82c3 to 13c9de2 Compare January 23, 2020 02:02
@jpobst jpobst marked this pull request as ready for review January 23, 2020 14:38
@jpobst jpobst requested a review from jonpryor as a code owner January 23, 2020 14:38
@jpobst jpobst requested a review from pjcollins January 23, 2020 14:38
Copy link
Contributor

@dellis1972 dellis1972 left a comment

Choose a reason for hiding this comment

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

This looks ok.

Copy link
Member

@pjcollins pjcollins left a comment

Choose a reason for hiding this comment

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

I'm slightly worried about test environment setup eating in to the parallel execution benefits. Right now it looks like we are adding 45 minutes or so to our overall execution time, though this does shave off roughly an hour in sequential run time.

Before:

  • TimeZoneInfo With Emulator - macOS: Duration: 2h 1m 31s

After:

  • TimeZoneInfo - macOS - 1: Duration: 55m 29s
  • TimeZoneInfo - macOS - 2: Duration: 55m 23s
  • TimeZoneInfo - macOS - 3: Duration: 55m 3s

Since these changes also ensure that we run this test job less frequently I think the increase in overall machine time is acceptable. We should look to improve the test environment provisioning costs elsewhere in the future though.

@jpobst jpobst merged commit 3e0b4da into master Jan 27, 2020
@jpobst jpobst deleted the parallel-timezoneinfo branch January 27, 2020 16:33
@github-actions github-actions bot locked and limited conversation to collaborators Jan 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants