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

Add support for expanding application on charge objects #1074

Merged
merged 1 commit into from
Dec 26, 2017

Conversation

ob-stripe
Copy link
Contributor

r? @remi-stripe
cc @stripe/api-libraries

Add support for expanding application attribute on charge objects

Fixes #1073.

public void it_should_have_connected_app_on_charge()
{
_charge.ApplicationId.Should().StartWith("ca_");
_charge.Application.Should().NotBeNull();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is test order execution guaranteed in XUnit? I'd expect to see the charge initialization in a setup method or a new charge explicitly created for this test.

Copy link
Contributor

Choose a reason for hiding this comment

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

agreed, usually this is done in a fixture first no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not entirely sure what you two meant, or if you're even talking about the same thing, but:

  • no, XUnit does not guarantee test order by default
  • using a fixture allows for context sharing between tests (cf. https://xunit.github.io/docs/shared-context.html). I updated the test to use a fixture to avoid creating multiple resources needlessly and make the tests a bit faster

@fred-stripe
Copy link
Contributor

Ah, makes sense @ob-stripe, I was thrown because the class and constructor name were snake_case and was wondering how connect_fixture() would get called.

lgtm

@ob-stripe ob-stripe merged commit 5074afd into master Dec 26, 2017
@ob-stripe ob-stripe deleted the ob-fix-1073 branch December 26, 2017 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants