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

Disable Topups tests as it breaks the Test suite because it's brittle #1192

Merged
merged 1 commit into from
May 23, 2018

Conversation

remi-stripe
Copy link
Contributor

Topups creation requires a Source to be chargeable. In Test mode this can take a random time though which leads to sporadic/random build failures.

For now we disable the tests as the resource works already and is still in private beta anyway.

Fixes #1110 (comment)

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

@brandur-stripe
Copy link
Contributor

Good call.

Minor nit: instead of commenting everything out, what do you think about skipping the topup tests? The advantage here is that we get a basic guarantee that at least the tests still compile (even if they don't run correctly), which is kind of nice.

It should be possible skip them with the Skip attribute on Fact:

[Fact(Skip="Feature is private beta and tests are non-deterministic")]

@remi-stripe remi-stripe force-pushed the remi-disable-topups-tests branch from d529649 to e434224 Compare May 23, 2018 14:06
@remi-stripe
Copy link
Contributor Author

ah neat, I did not know about that feature but I confirmed the tests are skipped locally!

PTAL

@remi-stripe
Copy link
Contributor Author

ah no I lied. This still creates a topup source in the fixtures I think.

@remi-stripe
Copy link
Contributor Author

Yeah I confirmed in my logs, every time I run that test suite it still creates the topup (or tries) so the issue is still here.

Do you have another idea?

@brandur-stripe
Copy link
Contributor

ah no I lied. This still creates a topup source in the fixtures I think.

Ah, dang. Maybe just minimize the commenting then by just commenting out the contents of public topups_fixture() in src/Stripe.Tests.XUnit/topups/_fixture.cs? I guess that's still a little better.

Topups creation requires a Source to be chargeable. In Test mode this can
take a random time though which leads to sporadic/random build failures.

For now we disable the tests as the resource works already and is still
in private beta anyway.
@remi-stripe remi-stripe force-pushed the remi-disable-topups-tests branch from e434224 to 928a4a9 Compare May 23, 2018 14:16
@remi-stripe
Copy link
Contributor Author

Okay, done by commenting the minimum amount of code in the fixture. PTAL

@brandur-stripe
Copy link
Contributor

LGTM. Thanks.

@brandur-stripe brandur-stripe merged commit c27fae5 into master May 23, 2018
@brandur-stripe brandur-stripe deleted the remi-disable-topups-tests branch May 23, 2018 14:44
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