Skip to content

fix(auth): Ensuring test user account cleanup with a Rule - #409

Merged
hiranya911 merged 2 commits into
idp-configfrom
hkj-user-cleanup
May 12, 2020
Merged

fix(auth): Ensuring test user account cleanup with a Rule#409
hiranya911 merged 2 commits into
idp-configfrom
hkj-user-cleanup

Conversation

@hiranya911

Copy link
Copy Markdown
Contributor

Using a JUnit @Rule to ensure that temporary user accounts are cleaned up in integration tests. This helps us avoid large try-finally blocks in tests.

I also moved couple of new tenant management tests to a new TenantManagerIT class.

@micahstairs micahstairs left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good! This is much better than try blocks.

Comment thread src/test/java/com/google/firebase/auth/TenantManagerIT.java Outdated
auth = FirebaseAuth.getInstance(masterApp);
}
@Rule
public final TemporaryUser temporaryUser = new TemporaryUser();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are you planning on adding a similar rule for tenants or should I take care of that?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I might as well do it so that we don't step on each others' toes. I'm also planning on doing this for the provider configs.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I didn't find that many instances where we create and delete provider configs. But if that's something we do repeatedly, then might be useful to make a similar change.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's not only a matter of reducing how many times we repeat the setup/cleanup. As it currently stands, if an assertion fails right after creating the provider config, it hasn't entered the "try" block yet, so the "finally" won't be executed. So we would either need to extend the try block back further, or we can use temporary provider configs. The temporary objects are nice because it is less of a burden on the developer writing integration tests, and it reduces the required nesting.

@lahirumaramba lahirumaramba left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks! LGTM!

@hiranya911
hiranya911 merged commit bfac46a into idp-config May 12, 2020
@hiranya911
hiranya911 deleted the hkj-user-cleanup branch May 12, 2020 18:37
micahstairs pushed a commit that referenced this pull request Jun 12, 2020
* fix(auth): Ensuring test user account cleanup with a Rule

* Updated copyright holder
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.

3 participants