Skip to content

Conversation

@adamsitnik
Copy link
Member

There was one warning, about the usage of Array.CreateInstance. IMO we can't get rid of it until we implement #1638

I've disabled this particular warning just to make sure we don't add any new warnings in the meantime. It's not perfect, but still better than no tests at all.

@adamsitnik
Copy link
Member Author

@jonsequitur the Ubuntu leg is most likely failing due to lack of native dependencies:

https://github.com/dotnet/runtime/blob/main/src/coreclr/nativeaot/docs/prerequisites.md

Should we add these dependencies or disable the test for this OS?

@adamsitnik
Copy link
Member Author

@jonsequitur there are other issues that we need to consider:

  1. contributors who don't have the https://github.com/dotnet/runtime/blob/main/src/coreclr/nativeaot/docs/prerequisites.md installed are going to get test failures
  2. it takes a lot of time to build an AOT app
  3. the test uses latest version of NativeAOT compiler. If there is a new bug introduced to it, the CI might suddenly start failing. It happened to us in BDN and we actually have two kind of tests: using hardcoded version to ensure to regressions in BDN and using latest to ensure it's supported

@jonsequitur
Copy link
Contributor

@jonsequitur the Ubuntu leg is most likely failing due to lack of native dependencies:

https://github.com/dotnet/runtime/blob/main/src/coreclr/nativeaot/docs/prerequisites.md

Should we add these dependencies or disable the test for this OS?

I'd prefer to add the dependencies. I'll look into it.

@jonsequitur
Copy link
Contributor

@jonsequitur there are other issues that we need to consider:

  1. contributors who don't have the https://github.com/dotnet/runtime/blob/main/src/coreclr/nativeaot/docs/prerequisites.md installed are going to get test failures
  2. it takes a lot of time to build an AOT app

We've been using ReleaseBuildOnlyFact and ReleaseBuildOnlyTheory for tests that are important to run during CI but would cause friction during the dev loop, such as complex setup or tests that are necessarily slow.

  1. the test uses latest version of NativeAOT compiler. If there is a new bug introduced to it, the CI might suddenly start failing. It happened to us in BDN and we actually have two kind of tests: using hardcoded version to ensure to regressions in BDN and using latest to ensure it's supported

My thought is the hardcoded version to prevent regression is simpler and more important, since here we're testing System.CommandLine and not the AOT compiler. Ingesting new dependencies dynamically can break this CI pipeline, so I would tend to avoid that.

adamsitnik and others added 3 commits February 25, 2022 18:42
@jonsequitur
Copy link
Contributor

@adamsitnik The update to Ubuntu 20.04 is merged. Could you rebase this and see if the tests now pass?

@adamsitnik
Copy link
Member Author

The update to Ubuntu 20.04 is merged. Could you rebase this and see if the tests now pass?

@jonsequitur I've rebased the branch but the test is till failing on Ubuntu. Is this repo using CI machines from MS internal feed? Or some public ones and we can define what is needed?

This is what we needed in BDN:

https://github.com/dotnet/BenchmarkDotNet/blob/ca51035a62ea89de6059c2822c4a5e5df58560a3/azure-pipelines.Ubuntu.yml#L17-L22

@jonsequitur
Copy link
Contributor

Do you want to try including that in your PR?

@adamsitnik adamsitnik requested a review from jonsequitur March 16, 2022 13:37
@adamsitnik
Copy link
Member Author

@jonsequitur it's finally green!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants