Skip to content

add sb test for runtime packs in self-contained publish#16783

Merged
MichaelSimons merged 7 commits intodotnet:mainfrom
oleksandr-didyk:add-self-contained-sb-test
Jul 18, 2023
Merged

add sb test for runtime packs in self-contained publish#16783
MichaelSimons merged 7 commits intodotnet:mainfrom
oleksandr-didyk:add-self-contained-sb-test

Conversation

@oleksandr-didyk
Copy link
Copy Markdown

Resolves dotnet/source-build#3485

Added test for source of runtime packs used in a self-contained publish.

The test creates a new project from the WebApp template (since this template would have dependency on both runtime packs currently distributed with the SDK) and does a self-contained publish. For the publish it will direct NuGet to use an empty cache. If the runtime packs are pulled from NuGet, they will end up in the cache, failing the test

@oleksandr-didyk oleksandr-didyk requested a review from a team as a code owner June 22, 2023 13:59
Copy link
Copy Markdown
Member

@MichaelSimons MichaelSimons left a comment

Choose a reason for hiding this comment

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

Have you validated the test fails if the packages are not bundled into the SDK?

@oleksandr-didyk
Copy link
Copy Markdown
Author

Have you validated the test fails if the packages are not bundled into the SDK?

Yes, I have, but only locally / in Codespaces, have not done that in CI

@omajid
Copy link
Copy Markdown
Member

omajid commented Jun 22, 2023

FYI @tmds

@oleksandr-didyk oleksandr-didyk force-pushed the add-self-contained-sb-test branch 2 times, most recently from 1ea4209 to 883988a Compare June 28, 2023 07:55
@oleksandr-didyk oleksandr-didyk force-pushed the add-self-contained-sb-test branch from 883988a to c63dd8b Compare July 18, 2023 10:24
@oleksandr-didyk oleksandr-didyk force-pushed the add-self-contained-sb-test branch from c63dd8b to 6bb37ee Compare July 18, 2023 10:28
Copy link
Copy Markdown
Member

@MichaelSimons MichaelSimons left a comment

Choose a reason for hiding this comment

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

I like the simplicity of this approach and the flexibility it adds to do custom validation down the road.

Assert.True("[]" == restoredPackageFiles, failMessage);
} else
{
throw new NugetCacheParseException();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think this warrants a new exception type. The primary reason is that nobody needs a specific type to catch. Additionally instead of incurring the overhead of throwing an exception, consider utilizing an Assert.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I normally try to utilize custom exceptions as they allow for distinguishing between test failures reported by asserts and some preparation logic like parsing an input file.

But I do agree it creates overhead and is a bit too complicated for this case. Removed

@MichaelSimons MichaelSimons merged commit a7815af into dotnet:main Jul 18, 2023
@oleksandr-didyk oleksandr-didyk deleted the add-self-contained-sb-test branch September 26, 2023 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add test for self-contained

3 participants