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

Update mac Helix queues #64565

Merged
merged 4 commits into from
Feb 3, 2022
Merged

Update mac Helix queues #64565

merged 4 commits into from
Feb 3, 2022

Conversation

agocke
Copy link
Member

@agocke agocke commented Jan 31, 2022

No description provided.

@agocke agocke requested review from sbomer and safern January 31, 2022 21:08
@ghost ghost assigned agocke Jan 31, 2022
@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.

- OSX.1014.Amd64.Open
- ${{ if or(ne(parameters.jobParameters.isExtraPlatforms, true), eq(parameters.jobParameters.includeAllPlatforms, true)) }}:
- OSX.1015.Amd64.Open
- OSX.1200.Amd64.Open
Copy link
Member

Choose a reason for hiding this comment

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

Since we no longer have a diff between extra platforms for OSX_x64 I would recommend removing the OSX_64 legs that run libraries tests:

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not opposed, but I'd rather do it in a different PR, in case it causes unrelated problems.

Copy link
Member

Choose a reason for hiding this comment

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

What worries me to delay it to a different PR is that usually those TODOs get delayed or slip on the backlog and we will just be wasting resources that are useful on the extra-platforms run.

Copy link
Member

Choose a reason for hiding this comment

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

Or at least if it is a subsequent PR you'll do in the next day or so, I don't mind.

@agocke
Copy link
Member Author

agocke commented Feb 1, 2022

@elinor-fung @AaronRobinsonMSFT @jkoritzinsky would you mind looking at this PR? After I upgraded the tests to use the latest Mac machines, it looks like some DllImport unit tests are failing

@elinor-fung
Copy link
Member

It looks like that test case is relying on setting LD_LIBRARY_PATH.

<CLRTestBashEnvironmentVariable Include="export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$%28pwd)/Subdirectory" />

Maybe this is that macOS security thing where they purge LD_LIBRARY_PATH (and DYLD_LIBRARY_PATH) when starting child processes? If it is that, I'd be fine just not running the TestNativeLibraryProbingOnPathEnv case on macOS:

@agocke
Copy link
Member Author

agocke commented Feb 2, 2022

Sounds good, I'll skip the test.

@steveisok
Copy link
Member

Is the OSX failure related? If not, is this ready to go?

@akoeplinger
Copy link
Member

Only the OSX.1014.Amd64 queue was removed so we could keep the 10.15 ones.

@premun
Copy link
Member

premun commented Feb 3, 2022

FYI there is this overview - https://github.com/dotnet/core-eng/wiki/Mobile-devices-for-.NET-testing (there are almost all OSX queues listed there, not just mobile)

The 12.00 is the largest queue by far with zero load, so we should definitely move some of the 10.14 workloads there (if possible). 10.15 is the most stressed queue at the moment if I am not mistaken.

@premun
Copy link
Member

premun commented Feb 3, 2022

Btw we have a related discussion ongoing - do we need docker on OSX queues?

@agocke
Copy link
Member Author

agocke commented Feb 3, 2022

@tommcdon It looks like a few profiler tests are failing on the new MacOS version. Could someone on your team take a look?

@safern
Copy link
Member

safern commented Feb 3, 2022

@agocke can we disable the test and open an issue in order to unblock other PRs?

@agocke
Copy link
Member Author

agocke commented Feb 3, 2022

@safern Yes, I didn't realize other PRs were failing. I'll do this immediately.

@agocke
Copy link
Member Author

agocke commented Feb 3, 2022

This is failing every PR so I don't think it can get worse. I'm going to merge now.

@ghost ghost locked as resolved and limited conversation to collaborators Mar 10, 2022
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.

7 participants