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

Adding test case for aggregate exception #147

Merged
merged 7 commits into from
Feb 17, 2018
Merged

Adding test case for aggregate exception #147

merged 7 commits into from
Feb 17, 2018

Conversation

RyanDawkins
Copy link

This PR is a remake of #142 to pull to master instead of the PassExceptionsOnTaskFaults branch.

@cleemullins
Copy link
Contributor

Looking now. Thanks!

@RyanDawkins
Copy link
Author

After pulling in your changes and the changes to TestAdapter by @drub0y , my test required no changes. Let me know if you notice anything off!

Copy link
Contributor

@cleemullins cleemullins left a comment

Choose a reason for hiding this comment

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

This is slick. I've never see the DataRow attribute used before.

@@ -154,5 +156,26 @@ await adapter
.AssertReply("three")
.StartTest();
}

[TestMethod]
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this supposed to be DataTestMethod?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Never mind. I tested the code. It worked fine as-is.

@@ -154,5 +156,26 @@ await adapter
.AssertReply("three")
.StartTest();
}

[TestMethod]
Copy link
Contributor

Choose a reason for hiding this comment

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

Never mind. I tested the code. It worked fine as-is.

@cleemullins cleemullins merged commit 2a41914 into microsoft:master Feb 17, 2018
@cleemullins
Copy link
Contributor

Thanks Ryan! I think this is a great addition to the tests.

Already tonight, with the Exception improvements to the Test Adapter, I've caught bugs that previously were slipping through. Having the exceptions flow through correctly has been a huge help!

@RyanDawkins
Copy link
Author

@cleemullins Sweet. I'll try and monitor the issues if you post them.

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.

2 participants