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

[ci] Remove separate SmokeTests jobs. #7872

Merged
merged 1 commit into from
Mar 14, 2023
Merged

[ci] Remove separate SmokeTests jobs. #7872

merged 1 commit into from
Mar 14, 2023

Conversation

jpobst
Copy link
Contributor

@jpobst jpobst commented Mar 10, 2023

Context: #7850 (comment)

With the re-parallelization of our test suites (#7804, #7850), combined with the fact that most PR's end up running the full test suite anyways, the separate "Smoke Tests" jobs aren't really saving us much, if any, CI time. We feel we would be better off using those additional agents to run the full test suite faster.

This PR removes the separate "Smoke Tests" jobs and runs all tests in the same jobs.

Additionally, as the [Category ("SmokeTests")] NUnit attribute is now only used by the Windows build stage to run in-tree tests, it has been cut back to the absolute bare minimum, saving ~45 minutes. Note that all the tests will continue to be run against the Windows installer, this only runs fewer tests for the Windows in-tree tests.

@jpobst jpobst force-pushed the remove-smoke-tests branch 3 times, most recently from 17f5d46 to 0d62b6c Compare March 13, 2023 23:48
@jpobst jpobst force-pushed the remove-smoke-tests branch from 0d62b6c to d265580 Compare March 14, 2023 16:04
@jpobst jpobst marked this pull request as ready for review March 14, 2023 20:04
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.

Looks good, if not for emulator flakiness our PR wait time is getting pretty close to 90 minutes which is great!

Comment on lines -466 to -474
# MSBuild Smoke Tests - Win
- template: yaml-templates/run-msbuild-tests.yaml
parameters:
testOS: Windows
jobName: win_smoke_msbuild_tests
jobDisplayName: Windows > Tests > MSBuild
agentCount: 2
testFilter: cat = SmokeTests
installApkDiff: true
Copy link
Member

Choose a reason for hiding this comment

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

I think this step built the repo on Windows. Would we lose anything there?

Could we potentially break the build on Windows and not know until someone tries it?

Copy link
Contributor Author

@jpobst jpobst Mar 14, 2023

Choose a reason for hiding this comment

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

I believe you are referring to the Windows build + run some in-tree tests on it, which is this stage:

https://github.com/xamarin/xamarin-android/blob/d26558092b0fe5edb6974bb8e59e317f147edfa6/build-tools/automation/azure-pipelines.yaml#L90

It will still run here.

image

The tests that were previously built by the job(s) you referenced are simply moved to the MSBuild stage Windows test.

Copy link
Member

Choose a reason for hiding this comment

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

The windows build stage (and testing of in-tree build artifacts rather than nugets) is part of https://github.com/xamarin/xamarin-android/blob/452e742e214cad42f2f30ea74097e422e49c04e3/build-tools/automation/yaml-templates/build-windows.yaml and will still be covered. This job was only running SmokeTests from Xamarin.Android.Build.Tests


- template: run-xaprepare.yaml
parameters:
arguments: --s=DetermineApplicableTests
Copy link
Member

Choose a reason for hiding this comment

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

So we could probably delete --s=DetermineApplicableTests in xaprepare, but maybe we don't have to do that here.

@jpobst jpobst merged commit 6508203 into main Mar 14, 2023
@jpobst jpobst deleted the remove-smoke-tests branch March 14, 2023 21:22
grendello added a commit to grendello/xamarin-android that referenced this pull request Mar 15, 2023
* main:
  [AOT] provide libc and libm stubs so NDK is not required (dotnet#7475)
  [MSBuildDeviceIntegration] Handle Debugger errors (dotnet#7864)
  [ci] Remove separate SmokeTests jobs. (dotnet#7872)
  Bump to dotnet/installer@cddf8e6 8.0.100-preview.3.23163.4 (dotnet#7885)
  [Actions] Do not double quote the commit message (dotnet#7887)
  [Actions] Ensure that the commit message is valid json. (dotnet#7884)
  [github] Use ".NET Android", not "Android for .NET" (dotnet#7882)
  Bump `$(AndroidNet7Version)` (dotnet#7883)
  [ci] Install fewer Android SDK platforms on test agents. (dotnet#7874)
grendello added a commit to grendello/xamarin-android that referenced this pull request Mar 15, 2023
* main:
  [AOT] provide libc and libm stubs so NDK is not required (dotnet#7475)
  [MSBuildDeviceIntegration] Handle Debugger errors (dotnet#7864)
  [ci] Remove separate SmokeTests jobs. (dotnet#7872)
  Bump to dotnet/installer@cddf8e6 8.0.100-preview.3.23163.4 (dotnet#7885)
  [Actions] Do not double quote the commit message (dotnet#7887)
  [Actions] Ensure that the commit message is valid json. (dotnet#7884)
  [github] Use ".NET Android", not "Android for .NET" (dotnet#7882)
  Bump `$(AndroidNet7Version)` (dotnet#7883)
  [ci] Install fewer Android SDK platforms on test agents. (dotnet#7874)
grendello added a commit to grendello/xamarin-android that referenced this pull request Mar 15, 2023
* main:
  [AOT] provide libc and libm stubs so NDK is not required (dotnet#7475)
  [MSBuildDeviceIntegration] Handle Debugger errors (dotnet#7864)
  [ci] Remove separate SmokeTests jobs. (dotnet#7872)
  Bump to dotnet/installer@cddf8e6 8.0.100-preview.3.23163.4 (dotnet#7885)
  [Actions] Do not double quote the commit message (dotnet#7887)
  [Actions] Ensure that the commit message is valid json. (dotnet#7884)
@github-actions github-actions bot locked and limited conversation to collaborators Jan 22, 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.

3 participants