-
Notifications
You must be signed in to change notification settings - Fork 19
test: parallel execution of e2e tests #1129
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
base: main
Are you sure you want to change the base?
Conversation
Reviewer's GuideThis PR enables true parallel execution of e2e tests by removing the Sequence Diagram: Mutex for Cosign Verification in Parallel TestssequenceDiagram
participant E2ETest1 as E2E Test 1
participant E2ETest2 as E2E Test 2
participant FileMutex
participant CosignProcess as Cosign Verification
E2ETest1 ->> FileMutex: Lock()
FileMutex -->> E2ETest1: Lock Acquired
E2ETest1 ->> CosignProcess: Verify()
CosignProcess -->> E2ETest1: Result
E2ETest1 ->> FileMutex: Unlock()
FileMutex -->> E2ETest1: Unlocked
E2ETest2 ->> FileMutex: Lock()
alt Test 1 holds or just released lock
FileMutex -->> E2ETest2: Lock Acquired (may involve waiting if Test 1 held lock)
else Lock was free
FileMutex -->> E2ETest2: Lock Acquired
end
E2ETest2 ->> CosignProcess: Verify()
CosignProcess -->> E2ETest2: Result
E2ETest2 ->> FileMutex: Unlock()
FileMutex -->> E2ETest2: Unlocked
Class Diagram: CosignVerifier Update with FileMutex DependencyclassDiagram
class CosignVerifier {
-mutex: FileMutex
+VerifySignature() error // Modified to use mutex
}
class FileMutex {
<<dependency: github.com/alexflint/go-filemutex>>
+New(filePath: string) *FileMutex
+Lock() error
+Unlock() error
}
CosignVerifier ..> FileMutex : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @osmman - I've reviewed your changes - here's some feedback:
- Rather than panicking on file mutex initialization, return an error or fail the test with a clear message so setup failures are more obvious.
- Don’t ignore errors from m.Lock() and m.Unlock(); handle and surface them to avoid silent deadlocks or unlock failures.
- For distributed test runs, consider using a cluster‐aware lock (e.g., a ConfigMap or leader election) instead of a local file lock for more robust synchronization.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
54032cd
to
1419c40
Compare
7193910
to
1d92bc6
Compare
37d4928
to
90f55c9
Compare
90f55c9
to
b228566
Compare
Summary by Sourcery
Enable parallel execution of end-to-end tests by removing the
-p 1
limitation and introducing a file-based mutex to serialize Cosign verification, preventing race conditions.New Features:
VerifyByCosign
to prevent concurrent access issues.Enhancements:
-p 1
flag from the Makefile test command.Build:
github.com/alexflint/go-filemutex v1.3.0
as a dependency for filesystem locking.