Skip to content

Validate FunctionId does not contain duplicate values#43608

Merged
jasonmalinowski merged 1 commit intodotnet:masterfrom
sharwell:no-duplicate-functions
Apr 23, 2020
Merged

Validate FunctionId does not contain duplicate values#43608
jasonmalinowski merged 1 commit intodotnet:masterfrom
sharwell:no-duplicate-functions

Conversation

@sharwell
Copy link
Contributor

Fixes #43607

@sharwell sharwell requested a review from a team as a code owner April 23, 2020 17:32
var value = (FunctionId)Enum.Parse(typeof(FunctionId), name);
if (map.TryGetValue(value, out var existingName))
{
Assert.True(false, $"'{nameof(FunctionId)}.{name}' cannot have the same value as '{nameof(FunctionId)}.{existingName}'");
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Assert.True(false, $"'{nameof(FunctionId)}.{name}' cannot have the same value as '{nameof(FunctionId)}.{existingName}'");
throw new Exception($"'{nameof(FunctionId)}.{name}' cannot have the same value as '{nameof(FunctionId)}.{existingName}'");

Otherwise you get a lot of head scratching as to why we expected true and got false.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the official xUnit pattern for Assert.Fail.
https://xunit.net/docs/comparisons.html

Copy link
Member

Choose a reason for hiding this comment

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

Oh really I had always thought the advice was indeed just throw.

@jasonmalinowski jasonmalinowski merged commit 838bd1b into dotnet:master Apr 23, 2020
@ghost ghost added this to the Next milestone Apr 23, 2020
@jasonmalinowski
Copy link
Member

Overriding the CI checks as we're seeing some failures do this this; there more things to unpack but this we know needs to be fixed regardless.

@sharwell sharwell deleted the no-duplicate-functions branch April 23, 2020 20:48
@jinujoseph jinujoseph added Area-IDE Test-Gap Describes a specific feature or scenario that does not have test coverage labels Apr 23, 2020
@sharwell sharwell modified the milestones: Next, temp, 16.7.P1 Apr 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-IDE Test-Gap Describes a specific feature or scenario that does not have test coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FunctionId cannot contain duplicate values

5 participants