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

Make tests fail when packages to test are not found locally (even if they are found online) #3771

Closed
dagood opened this issue Sep 20, 2019 · 2 comments
Labels

Comments

@dagood
Copy link
Member

dagood commented Sep 20, 2019

Steps to reproduce

Build locally with tests, using a build number matching a completed official build's number, with these lines removed:

https://github.com/dotnet/core-setup/blob/65dc0465be93850df24f444010491dc2e53127f2/NuGet.config#L9-L13

Expected behavior

The tests fail because they are unable to find the packages it just built.

Actual behavior

The tests succeed, using packages that the official build published. This causes a PR to show up green even though only that very specific build number would pass.


This caused an auto-update PR to be merged when it shouldn't have. Info at dotnet/core-setup#8296. However, it seems like an unlikely test bug, removing those lines from the NuGet.config is a Maestro++/Darc bug that is getting some regression tests.

A solution might be to remove the Core-Setup dev source from the list to check during tests, and rely more on CopyPotentiallyInternalPackagesForTestRestore, which wouldn't include the packages that made test restore work:

https://github.com/dotnet/core-setup/blob/c8a9346a88eab4726cc74b4e3e2354fdf67b3999/src/test/PrepareTestAssets/PrepareTestAssets.proj#L95-L103

@dagood
Copy link
Member Author

dagood commented Sep 26, 2019

IMO we should treat this as a relatively high priority test infra fix in 3.0. It is not good to be unsure whether green CI is actually ok, and it wastes time when investigating other issues to wonder if this problem is occurring.

With servicing builds needing to edit nuget.config a bunch, this could have an identical repro if there's some Darc regression or new bug.

/cc @mmitche @MichaelSimons @dleeapho

@dagood
Copy link
Member Author

dagood commented Oct 30, 2019

Closing: this only gets the bad change past the PR validation, but it fails in the official build. It's still relatively close to the cause so not as much of a nightmare to debug as I was thinking.

@dagood dagood closed this as completed Oct 30, 2019
@msftgits msftgits transferred this issue from dotnet/core-setup Jan 30, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

1 participant