Skip to content

Fix domainscrawl race in tests#459

Merged
CorentinB merged 1 commit intomainfrom
fix-domainscrawl-race
Sep 3, 2025
Merged

Fix domainscrawl race in tests#459
CorentinB merged 1 commit intomainfrom
fix-domainscrawl-race

Conversation

@CorentinB
Copy link
Copy Markdown
Collaborator

This pull request refactors the domainscrawl matcher in internal/pkg/postprocessor/domainscrawl/domainscrawl.go to improve testability and encapsulation. The matcher logic is moved from global functions and variables to instance methods on a new matchEngine type, and the tests are updated to use matcher instances instead of global state. This makes the codebase more modular and easier to test in isolation.

Matcher encapsulation and API changes:

  • Introduced a matchEngine type and moved matcher logic from global functions to instance methods (e.g., AddElements, Match, Reset, Enabled). Added a NewMatcher constructor for creating isolated matcher instances. (internal/pkg/postprocessor/domainscrawl/domainscrawl.go)
  • Updated the matcher API so that all matcher operations (add, match, reset, enabled) are now performed on a matchEngine instance, rather than via global functions and state. (internal/pkg/postprocessor/domainscrawl/domainscrawl.go) [1] [2] [3] [4]

Test improvements:

  • Refactored all tests in domainscrawl_test.go to use local matchEngine instances created with NewMatcher, removing reliance on global state and making tests more robust and parallelizable. (internal/pkg/postprocessor/domainscrawl/domainscrawl_test.go) [1] [2] [3] [4] [5] [6] [7] [8]

@CorentinB CorentinB self-assigned this Sep 3, 2025
@CorentinB CorentinB added the bug Something isn't working label Sep 3, 2025
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the domainscrawl matcher to eliminate race conditions in tests by moving from global state to instance-based design. The matcher logic is encapsulated in a matchEngine type with instance methods, improving testability and thread safety.

Key changes:

  • Introduced matchEngine type with instance methods (AddElements, Match, Reset, Enabled) replacing global functions
  • Added NewMatcher() constructor for creating isolated matcher instances
  • Updated all tests to use local matcher instances instead of global state

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
internal/pkg/postprocessor/domainscrawl/domainscrawl.go Refactored matcher from global functions to instance methods on matchEngine type
internal/pkg/postprocessor/domainscrawl/domainscrawl_test.go Updated tests to use local matcher instances created with NewMatcher()

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread internal/pkg/postprocessor/domainscrawl/domainscrawl.go
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 79.06977% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.20%. Comparing base (5282aef) to head (d2b9b54).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...nal/pkg/postprocessor/domainscrawl/domainscrawl.go 79.06% 9 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #459      +/-   ##
==========================================
- Coverage   56.20%   56.20%   -0.01%     
==========================================
  Files         130      130              
  Lines        8022     8037      +15     
==========================================
+ Hits         4509     4517       +8     
- Misses       3154     3161       +7     
  Partials      359      359              
Flag Coverage Δ
e2etests 40.50% <46.51%> (+0.03%) ⬆️
unittests 29.24% <69.76%> (-0.07%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@CorentinB CorentinB merged commit 4ab3e84 into main Sep 3, 2025
5 checks passed
@CorentinB CorentinB deleted the fix-domainscrawl-race branch September 3, 2025 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants