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 matrix change: Windows #58989

Merged
merged 15 commits into from
Mar 28, 2022
Merged

Conversation

aik-jahoda
Copy link
Contributor

@aik-jahoda aik-jahoda commented Sep 11, 2021

Changes in CI matrix:

OS What to do Notes
Windows 7 (x86) Drop PR (rolling sufficient)
Windows 8.1 Drop PR in favor of W10+
Windows 10 19H1 (server as proxy for client) (x86 and 64) As far as possible, remove x64 rolling in favor of W11.
Windows Server RS5 (1809) Drop it (EOL)

part of #57947

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

Copy link
Member

@wfurt wfurt left a comment

Choose a reason for hiding this comment

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

generally look ok to me. It is nice to see Windows7 go.

eng/pipelines/libraries/helix-queues-setup.yml Outdated Show resolved Hide resolved
@@ -128,46 +128,39 @@ jobs:
# netcoreapp
- ${{ if notIn(parameters.jobParameters.framework, 'net48') }}:
- ${{ if and(eq(parameters.jobParameters.testScope, 'outerloop'), eq(parameters.jobParameters.runtimeFlavor, 'mono')) }}:
- Windows.81.Amd64.Open
- Windows.10.Amd64.Server19H1.Open
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to keep this one in Rolling as well to measure pass rate of the pipeline and match PR with Rolling as much as possible?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

The original idea was to mix PR and rolling to save resources but with @jkotas's proposal, we will run rolling builds less often and should probably always the full matrix (of OS + arch + configuration) on rolling builds.

@aik-jahoda
Copy link
Contributor Author

@safern, @jkoritzinsky , are you ok with this matrix change?

@aik-jahoda aik-jahoda requested review from a team and jkotas September 30, 2021 13:58
- Windows.10.Amd64.Server19H1.Open
- ${{ if or(ne(parameters.jobParameters.testScope, 'outerloop'), ne(parameters.jobParameters.runtimeFlavor, 'mono')) }}:
- ${{ if eq(parameters.jobParameters.isFullMatrix, true) }}:
- Windows.81.Amd64.Open
- Windows.10.Amd64.ServerRS5.Open
- Windows.10.Amd64.Server19H1.Open
- Windows.10.Amd64.Server19H1.ES.Open
Copy link
Member

Choose a reason for hiding this comment

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

Could you leave the .ES. one on both PRs and rolling? We get english coverage on Windows.11.

FYI @danmoseley

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently Windows.10.Amd64.Server19H1.ES.Open is on both PR and Rolling. Or did I mess the isFullMatrix conditions?

Copy link
Member

Choose a reason for hiding this comment

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

Another option is to mix in a different locale. @AlexRadch has made several fixes to tests so they pass in ru-RU. Another contributor has fixed issues in ko-KR. Clearly ES isn't covering it all. So we could have JP or KO.

Up to you guys.

Copy link
Member

Choose a reason for hiding this comment

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

For that we would need a new helix queue. So we can get that request up to change the .ES. machines to another locale, I like that idea.

Copy link
Member

Choose a reason for hiding this comment

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

Currently Windows.10.Amd64.Server19H1.ES.Open is on both PR and Rolling. Or did I mess the isFullMatrix conditions?

You are right. I miss read the change.

@ghost
Copy link

ghost commented Nov 1, 2021

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Issue Details

Changes in CI matrix:

OS What to do Notes
Windows 7 (x86) Drop PR (rolling sufficient)
Windows 8.1 Drop PR in favor of W10+
Windows 10 19H1 (server as proxy for client) (x86 and 64) As far as possible, remove x64 rolling in favor of W11.
Windows Server RS5 (1809) Drop it (EOL)

part of #57947

Author: aik-jahoda
Assignees: -
Labels:

area-Infrastructure

Milestone: -

@karelz
Copy link
Member

karelz commented Nov 29, 2021

Update: Chasing down one last timeout problem. Using testing PR #61970 with additional legs.

@aik-jahoda
Copy link
Contributor Author

cc @eiriktsarpalis @layomia

Do you have any idea why the ForbidUndefinedCharacters_RemovesUndefinedChars fails? Are there any OS prerequisites for the test?

Console log: 'System.Text.Encodings.Web.Tests' from job 5e681f82-3efa-4a3e-8b11-6b0a8f255225 workitem eb33ae1d-02cf-4f87-ada1-9eb34701aab9 (windows.11.amd64.clientpre.open) executed on machine a002YFC

C:\h\w\AFB00942\w\BE220A24\e>taskkill.exe /f /im corerun.exe 
ERROR: The process "corerun.exe" not found.

C:\h\w\AFB00942\w\BE220A24\e>call RunTests.cmd --runtime-path C:\h\w\AFB00942\p 
----- start Mon 12/06/2021 15:46:18.12 ===============  To repro directly: ===================================================== 
pushd C:\h\w\AFB00942\w\BE220A24\e\
"C:\h\w\AFB00942\p\dotnet.exe" exec --runtimeconfig System.Text.Encodings.Web.Tests.runtimeconfig.json --depsfile System.Text.Encodings.Web.Tests.deps.json xunit.console.dll System.Text.Encodings.Web.Tests.dll -xml testResults.xml -nologo -nocolor -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing 
popd
===========================================================================================================

C:\h\w\AFB00942\w\BE220A24\e>"C:\h\w\AFB00942\p\dotnet.exe" exec --runtimeconfig System.Text.Encodings.Web.Tests.runtimeconfig.json --depsfile System.Text.Encodings.Web.Tests.deps.json xunit.console.dll System.Text.Encodings.Web.Tests.dll -xml testResults.xml -nologo -nocolor -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing  
  Discovering: System.Text.Encodings.Web.Tests (method display = ClassAndMethod, method display options = None)
  Discovered:  System.Text.Encodings.Web.Tests (found 212 test cases)
  Starting:    System.Text.Encodings.Web.Tests (parallel test collections = on, max threads = 2)
    System.Text.Encodings.Web.Tests.AllowedBmpCodePointsBitmapTests.ForbidUndefinedCharacters_RemovesUndefinedChars [FAIL]
      Assert.Equal() Failure
      Expected: True
      Actual:   False
      Stack Trace:
        /_/src/libraries/System.Text.Encodings.Web/tests/AllowedBmpCodePointsBitmapTests.cs(87,0): at System.Text.Encodings.Web.Tests.AllowedBmpCodePointsBitmapTests.ForbidUndefinedCharacters_RemovesUndefinedChars()
  Finished:    System.Text.Encodings.Web.Tests
=== TEST EXECUTION SUMMARY ===
   System.Text.Encodings.Web.Tests  Total: 652, Errors: 0, Failed: 1, Skipped: 0, Time: 4.336s
----- end Mon 12/06/2021 15:46:23.04 ----- exit code 1 ----------------------------------------------------------
2021-12-06T15:46:23.715Z	INFO   	run.py	run(48)	main	Beginning reading of test results.
2021-12-06T15:46:23.715Z	INFO   	run.py	__init__(48)	get_log_files	Searching 'C:\h\w\AFB00942\w\BE220A24\e\..' for log files
Found log 'C:\h\w\AFB00942\w\BE220A24\e\..\console.95c30e0d.log'
Uri 'https://helixre8s23ayyeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-heads-jajahoda-libmatrix-5e681f823efa4a3e8b/System.Text.Encodings.Web.Tests/console.95c30e0d.log?sv=2019-07-07&se=2021-12-26T15%3A34%3A41Z&sr=c&sp=rl&sig=OL7%2FjCWh8CqpIwnNF0vcXSssUrDYhx85alvT4QDHCqg%3D'
2021-12-06T15:46:23.715Z	INFO   	run.py	__init__(66)	construct_log_list	Generated log list: console.95c30e0d.log:
  https://helixre8s23ayyeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-heads-jajahoda-libmatrix-5e681f823efa4a3e8b/System.Text.Encodings.Web.Tests/console.95c30e0d.log?sv=2019-07-07&se=2021-12-26T15%3A34%3A41Z&sr=c&sp=rl&sig=OL7%2FjCWh8CqpIwnNF0vcXSssUrDYhx85alvT4QDHCqg%3D

2021-12-06T15:46:23.715Z	INFO   	run.py	__init__(90)	read_results	Searching 'C:\h\w\AFB00942\w\BE220A24\e' for test results files
2021-12-06T15:46:23.715Z	INFO   	run.py	__init__(96)	read_results	Found results file C:\h\w\AFB00942\w\BE220A24\e\testResults.xml with format xunit
2021-12-06T15:46:23.715Z	INFO   	run.py	__init__(90)	read_results	Searching 'C:\h\w\AFB00942\w\BE220A24\uploads' for test results files
2021-12-06T15:46:23.715Z	INFO   	run.py	packing_test_reporter(30)	report_results	Packing 652 test reports to 'C:\h\w\AFB00942\w\BE220A24\e\__test_report.json'
2021-12-06T15:46:23.731Z	INFO   	run.py	packing_test_reporter(33)	report_results	Packed 189433 bytes
Did not find dumps, skipping dump docs generation.

@danmoseley
Copy link
Member

@agocke is this good to merge? i fixed a conflict

@danmoseley
Copy link
Member

not sure whether I need to trigger any extra legs?

@danmoseley
Copy link
Member

@agocke ?

failure is only #65817 but maybe other legs need to be triggered?

@agocke
Copy link
Member

agocke commented Mar 28, 2022

Sorry, not sure why I missed this before. LGTM.

@agocke agocke merged commit bd43f55 into dotnet:main Mar 28, 2022
radekdoulik pushed a commit to radekdoulik/runtime that referenced this pull request Mar 30, 2022
* CI matrix change: Windows

* Remove Windows Server RS5

* Revert .net Framework change

* Add 19H1 to rolling

* Add Windows.Server.Core.20H2 to CI matrix

* Remove Windows 8.1

* Remove 20H2 as thhere is a PR for it dotnet#60054

* Typo

Co-authored-by: Jan Jahoda <jajahoda@microsoft.com>
Co-authored-by: Andy Gocke <andy@commentout.net>
Co-authored-by: Dan Moseley <danmose@microsoft.com>
@ghost ghost locked as resolved and limited conversation to collaborators Apr 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants