Skip to content
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

Replace [ExpectedException] with Assert.Throws #35

Merged
merged 1 commit into from
Nov 14, 2016
Merged

Replace [ExpectedException] with Assert.Throws #35

merged 1 commit into from
Nov 14, 2016

Conversation

stakx
Copy link
Contributor

@stakx stakx commented Nov 13, 2016

This commit replaces all occurrences of the [ExpectedException] attribute on unit test methods with the more recent and flexible Assert.Throws syntax. As outlined in issue #34, this has some benefits:

  1. improves separation between the Arrange and Act stages of Arrange-Act-Assert;
  2. improves test code consistency;
  3. removes an obstacle for migrating to more recent versions of NUnit.

As outlined in issue #34, unit tests relying on a `[ExpectedException]`
attribute should be rewritten to use `Assert.Throws` instead:

 * `[ExpectedException]` is too coarse-grained and doesn't allow us to
   test only the Act stage in Arrange-Act-Assert. `Assert.Throws` does
   not suffer from this.

 * `[ExpectedException]` is no longer supported in NUnit version 3.

This commit replaces all occurrences of the attribute with the more
flexible `Assert.Throws`. In some cases, parts of the `TestDelegate`
that belong to the Arrange stage are moved out of it.
@msftclas
Copy link

@stakx, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, MSBOT;

@doubleyewdee doubleyewdee merged commit d1b5880 into microsoft:master Nov 14, 2016
@stakx stakx deleted the issue-34-replace-expectedexception branch November 14, 2016 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants