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

feat: unit tests for link checking #10075

Merged
merged 1 commit into from
Sep 4, 2024
Merged

Conversation

hamza221
Copy link
Contributor

@hamza221 hamza221 commented Aug 30, 2024

requires #10073

Fixes #9453

@kesselb
Copy link
Contributor

kesselb commented Aug 30, 2024

Merge this one into the other branch? ;) The tests are failing, because the fix is not in, yet.

@hamza221
Copy link
Contributor Author

hamza221 commented Sep 2, 2024

Merge this one into the other branch? ;) The tests are failing, because the fix is not in, yet.

I can rebase and merge to main later. since they're unrelated PRs

@hamza221 hamza221 mentioned this pull request Sep 2, 2024
17 tasks
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Whitespaces in test are a bit random

One way to structure them is the A-A-A pattern

  • Arrange block
  • Blank line
  • Act block
  • Blank line
  • Assert block

This would fit nicely for the tests here.

https://automationpanda.com/2020/07/07/arrange-act-assert-a-pattern-for-writing-good-tests/

tests/Unit/Service/Phishing/LinkCheckTest.php Show resolved Hide resolved
tests/Unit/Service/Phishing/LinkCheckTest.php Show resolved Hide resolved
tests/Unit/Service/Phishing/LinkCheckTest.php Outdated Show resolved Hide resolved
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Noice

Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Ugh one more thing: make the commit a test:. This is not a user-facing feature 😬

@hamza221 hamza221 merged commit 629a9fa into main Sep 4, 2024
32 of 33 checks passed
@hamza221 hamza221 deleted the enh/unit-tests-link-checking branch September 4, 2024 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Phishing detection
3 participants