Skip to content

Conversation

@pentp
Copy link
Contributor

@pentp pentp commented Nov 19, 2019

A by-product of fixing https://github.com/dotnet/corefx/issues/5364 was that AggregateException.Flatten() will construct a new exception using the expanded message from the existing exception which will be expanded again when used.

The test diff for this change would have been the following:

-Assert.Equal("message (A) (B) (C) (A) (B) (C)", flattened1.Message);
+Assert.Equal("message (A) (B) (C)", flattened1.Message);
-Assert.Equal("message (message (A) (B) (C)) (message (A) (B) (C)) (A) (B) (C) (A) (B) (C)", flattened2.Message);
+Assert.Equal("message (A) (B) (C) (A) (B) (C)", flattened2.Message);

@stephentoub Is the check for exact type match OK or too much?

Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

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

Seems reasonable. Thanks.

However, the dotnet/runtime repo is still being enabled and we don't yet have the corefx tests running against a live built corelib; at this point, although they're in the same repo, we're still running the tests against a nuget package containing corelib. So the tests will need to be submitted in a separate PR.

@pentp
Copy link
Contributor Author

pentp commented Nov 20, 2019

Maybe it's better to wait for #71 to complete first. This PR could then be completed in one step?

@stephentoub
Copy link
Member

Whichever you prefer.

@pentp
Copy link
Contributor Author

pentp commented Dec 6, 2019

Created a new PR for this as the runtime repo was made public.

@pentp pentp closed this Dec 6, 2019
@karelz karelz added this to the 5.0.0 milestone Aug 18, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants