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

Fix missing assertions warning #3203

Merged
merged 1 commit into from
Oct 1, 2024
Merged

Fix missing assertions warning #3203

merged 1 commit into from
Oct 1, 2024

Conversation

yndajas
Copy link
Member

@yndajas yndajas commented Oct 1, 2024

We were getting a lot of noise about tests missing assertions because of using custom assertion methods (or just relying on tests not raising errors rather than explicitly asserting anything). This adds assert in multiple places to fix this, and in one case adds a default return of true in a method we need to assert on

E.g.

Test is missing assertions: `test_: setting a 2SV code signed in with an existing 2SV setup should require the code again on next login. ` /govuk/signon/test/integration/two_step_verification_test.rb:57

This application is owned by the publishing platform team. Please let us know in #govuk-publishing-platform when you raise any PRs.

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

Follow these steps if you are doing a Rails upgrade.

We were getting a lot of noise about tests missing assertions because of
using custom assertion methods (or just relying on tests not raising
errors rather than explicitly asserting anything). This adds `assert`
in multiple places to fix this, and in one case adds a default return of
`true` in a method we need to assert on

E.g.

```
Test is missing assertions: `test_: setting a 2SV code signed in with an existing 2SV setup should require the code again on next login. ` /govuk/signon/test/integration/two_step_verification_test.rb:57
```
Copy link
Contributor

@Gweaton Gweaton left a comment

Choose a reason for hiding this comment

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

Nice, well done for tidying this up.

@yndajas yndajas merged commit f68b44d into main Oct 1, 2024
16 checks passed
@yndajas yndajas deleted the fix-missing-assertions branch October 1, 2024 10: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