-
Notifications
You must be signed in to change notification settings - Fork 437
Refactor smoke-test to capture prereqs #13145
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
Refactor smoke-test to capture prereqs #13145
Conversation
|
These changes have passed PR validation and an internal build w/bootstrapping. |
crummel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small gotcha, otherwise LGTM.
| public static string DotNetDirectory { get; } = Environment.GetEnvironmentVariable("DOTNET_DIR") ?? "./.dotnet"; | ||
| public static string DotNetTarballPath { get; } = Environment.GetEnvironmentVariable(DotNetTarballPathEnv) ?? string.Empty; | ||
| public const string DotNetTarballPathEnv = "DOTNET_TARBALL_PATH"; | ||
| public static bool ExcludeOnlineTests { get; } = Environment.GetEnvironmentVariable("EXCLUDE_ONLINE_TESTS") != null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| public static bool ExcludeOnlineTests { get; } = Environment.GetEnvironmentVariable("EXCLUDE_ONLINE_TESTS") != null; | |
| public static bool ExcludeOnlineTests { get; } = Convert.ToBoolean(Environment.GetEnvironmentVariable("EXCLUDE_ONLINE_TESTS")); |
I think it's more intuitive to avoid the issue with the environment variable being EXCLUDE_ONLINE_TESTS=false causing them to be excluded.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair suggestion. I think I like avoiding exceptions for this scenario though so how about - bool.TryParse(Environment.GetEnvironmentVariable("EXCLUDE_ONLINE_TESTS"), out bool excludeOnlineTests) ? excludeOnlineTests : false;
Fixes dotnet/source-build#2550
./build.sh --run-smoke-testwill now captures all prereq packages downloaded from online sources that are not part of source-build into dotnet-smoke-test-prereqs.xyz.tar.gz in the tarball build output next to the other tarball artifacts. The smoke-tests will now run two passes. Once against online package and a second against source-build packages w/prereqs.