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

Use builders for token and account v2 service tests #304

Merged

Conversation

A-Guldborg
Copy link
Contributor

@A-Guldborg A-Guldborg commented Nov 26, 2024

This pull requests addresses @TTA777 review on #289 wrt using test data builders.

This PR ensures we use builders for our models during testing. Though for our tokens we need to give it our custom instantiator, as we would like to keep the property of having the Token type require the two parameters token hash and token type and using this information to set the default expired DateTime, but still allowing us to overwrite this time for multiple purposes (testing, specific purposes at a later stage etc.).

This PR also introduces integration tests for the new magic link authentication flow, only mocking the actual IEmailSender.

@A-Guldborg A-Guldborg requested a review from TTA777 November 26, 2024 23:16
@A-Guldborg A-Guldborg self-assigned this Nov 26, 2024
@TTA777
Copy link
Member

TTA777 commented Nov 28, 2024

Hmm, it seems to me that the simplest solution to your conundrum @A-Guldbor,g is to go for option two, and making the builder use the constructor you want.

I tried playing a bit locally, and the following works for me

public static TokenBuilder Simple()
{
var builder = new TokenBuilder();
            builder.Faker.CustomInstantiator(f => new Token("some token hash here, maybe use the fake to generate it", TokenType.Refresh));
            return builder
                .WithExpires(DateTime.Now.AddDays(1))
                .WithRevoked(false)
                .WithUser(UserBuilder.DefaultCustomer().Build());
}

I'm unsure at present what you dislike about this solution, since it seems we can keep using the builders the way we currently are, and you can keep the current code design.

My only alternative, off the top of my head, would be to:

  • Make the setter for Expires private and adding logic to the TokenType setter ensuring it sets the right expiration based on the type as well
  • Mark both the tokenHash and type as required as you started to do in this PR

This approach would probably have one problem though, being you wouldn't be able to manipulate the exipration of a token for a test. I'm unsure if that is something we would even want to do, but it sounds reasonably within the scope of things we might do in a test.

As to how the builders actually work, well the short answer is probably a bit of black magic. More seriously, it is making use of a source generator (our builderGenerator.cs), and a library called Bogus. The generator makes sure we have the WithPropteryName methods, and the bogus library can then be used for creating somewhat realistic test data when we want to, instead of being specific.

@A-Guldborg A-Guldborg changed the title Changing v2 account service tests to use builders Use builders for token and account v2 service tests Dec 3, 2024
@A-Guldborg A-Guldborg marked this pull request as ready for review December 3, 2024 18:13
@A-Guldborg A-Guldborg merged commit 6f79570 into feature/magic-link-login Dec 3, 2024
@A-Guldborg A-Guldborg deleted the fix/tests-with-builders-for-new-tokens branch December 3, 2024 18:23
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