Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Cleanup and add some CreditCardAttribute tests - #10993

Merged
stephentoub merged 2 commits into
dotnet:masterfrom
hughbe:creditcardattribute-tests
Aug 19, 2016
Merged

Cleanup and add some CreditCardAttribute tests#10993
stephentoub merged 2 commits into
dotnet:masterfrom
hughbe:creditcardattribute-tests

Conversation

@hughbe

@hughbe hughbe commented Aug 19, 2016

Copy link
Copy Markdown

No description provided.

attribute.ErrorMessage = null; // note: this overrides the default value
attribute.ErrorMessage = message;
attribute.ErrorMessageResourceName = resourceName;
attribute.ErrorMessageResourceType = resourceType;

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'm not sure this is actually testing the same thing as was there previously. Setting these to null isn't the same as not setting them, e.g.

_errorMessage = value;
_errorMessageResourceAccessor = null;
CustomErrorMessageSet = true;
// Explicitly setting ErrorMessage also sets DefaultErrorMessage if null.
// This prevents subsequent read of ErrorMessage from returning default.
if (value == null)
{
_defaultErrorMessage = null;
}

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 see - I've fixed this

@stephentoub

Copy link
Copy Markdown
Member

Other than my one comment, LGTM. Thanks for the cleanup.

@stephentoub

Copy link
Copy Markdown
Member

Test Innerloop CentOS7.1 Debug Build and Test please
Test Innerloop Ubuntu14.04 Release Build and Test please

@stephentoub
stephentoub merged commit 3f76ba2 into dotnet:master Aug 19, 2016
@hughbe
hughbe deleted the creditcardattribute-tests branch August 19, 2016 19:14
@karelz karelz modified the milestone: 1.1.0 Dec 3, 2016
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants