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

Test coverage seems low #468

Open
iandunn opened this issue Oct 10, 2022 · 3 comments · Fixed by #469
Open

Test coverage seems low #468

iandunn opened this issue Oct 10, 2022 · 3 comments · Fixed by #469

Comments

@iandunn
Copy link
Member

iandunn commented Oct 10, 2022

I ran an Xdebug report locally and the overall coverage was 34% before #427, and 23% after.

I'm not a coverage zealot, and don't think 100% is a reasonable, but 100% over the critical areas seems reasonable for a security plugin, especially before it becomes a canonical plugin or merges into Core.

sjinks/wp-two-factor-provider-webauthn has some e2e tests that might provide a head start. Adding more unit/integration tests would be nice too.

Using @codeCoverageIgnore on the untestable and low-severity areas would also help make it more obvious if everything that should be covered actually is.

Related: #467

This was referenced Oct 10, 2022
@jeffpaul jeffpaul linked a pull request Oct 12, 2022 that will close this issue
@jeffpaul jeffpaul added this to the 0.8.0 milestone Oct 12, 2022
@iandunn
Copy link
Member Author

iandunn commented Oct 12, 2022

i didn't mean to close this with #469 , that just increased TOTP. I think it's worth examining the rest of the plugin as well (except U2F).

@iandunn iandunn reopened this Oct 12, 2022
@iandunn
Copy link
Member Author

iandunn commented Oct 12, 2022

It's not necessarily a priority for 0.8, though, unless y'all want it to be.

@iandunn
Copy link
Member Author

iandunn commented Nov 18, 2022

Related #497

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants