Create Stripe customer factories, use in tests #634
Conversation
Deletes these fixtures: - stripe_customer_data - raw_stripe_customer_data - contact_with_stripe_customer In favor of factory usage in tests
grahamalama
changed the title
There was a problem hiding this comment.
The job running on runner GitHub Actions 16 has exceeded the maximum execution time of 5 minutes.
Seems like the tests might've ran a little longer than normal? (Or maybe this was a CI-hiccup?)
Does this run long on local as well? If not we can maybe just up the CI-build time?
| if not create: | ||
| return | ||
| if extracted: | ||
| FirefoxAccountFactory(email=self, **kwargs) |
There was a problem hiding this comment.
Nope -- since this is the EmailFactory, all we're doing here is creating the FirefoxAccount and associating it with this Email object. In a test, this would work fine:
email = EmailFactory()
dbsession.commit()
assert email.fxa ...
Unless you're saying that we should avoid the implicit return of None, in which case I could make that fix.
There was a problem hiding this comment.
I get it, email=self does it. Sorry for the confusion
| model = models.FirefoxAccount | ||
|
|
||
| fxa_id = factory.LazyFunction(lambda: uuid4().hex) | ||
| primary_email = factory.SelfAttribute("email.primary_email") |
There was a problem hiding this comment.
👌
Shall we add a comment that this simulates the DB relationship?
There was a problem hiding this comment.
I'm curious what you mean by "simulates"? By using this factory, we'll create an Email and FirefoxAccount object, and they'll be related through fxa_instance.email.
There was a problem hiding this comment.
I'm confused by my own comment...
I didn't know about SelfAttribute and looked up its docs to understand how it was leveraged. I first understood that we were linking the two fields using this lazy attribute, as if they were linked by foreign keys in the DB.
But actually, I see that primary_email in the FirefoxAccount model is not linked to email.primary_email at all, they can be different AFAIU. So scratch that
| ): | ||
| """A Stripe Invoice and related objects can return the related email_id.""" | ||
| customer = stripe_customer_factory(stripe_id=FAKE_STRIPE_CUSTOMER_ID) |
There was a problem hiding this comment.
Why do we set the id here?
There was a problem hiding this comment.
While we're in the middle of conversion, there are still a few places where we have "magic" object IDs. In this case, the way we use raw_stripe_invoice_data assumes that there's a a customer whose ID is FAKE_STRIPE_CUSTOMER_ID. So for now, that's what we specify when we generate the Stripe customer, but in a follow-up PR, we'll hopefully be able to remove this.
@bsieber-mozilla there does seem to be something wrong with the integration tests. I'll investigate further (#638) |
Ref: #633
Deletes these fixtures:
stripe_customer_dataraw_stripe_customer_datacontact_with_stripe_customerIn favor of factory usage in tests