fix: Update mocks for change to working dir locker interface#6248
fix: Update mocks for change to working dir locker interface#6248lukemassa wants to merge 2 commits intorunatlantis:mainfrom
Conversation
Signed-off-by: Luke Massa <lukefrederickmassa@gmail.com>
There was a problem hiding this comment.
Pull request overview
Updates the pegomock-generated WorkingDirLocker mock and a couple of unit-test verifications to match the TryLock(..., projectName, ...) interface change introduced in #6086, ensuring tests/mocks compile and verify calls use the correct arity.
Changes:
- Update
VerifyWasCalled(...).TryLock(...)usages in pre/post workflow hook runner tests to include the newprojectNameargument. - Regenerate/update
MockWorkingDirLockerverifier/captured-args helpers to match the newTryLocksignature. - Align captured argument extraction in the mock to include
projectNameas a string parameter.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
server/events/pre_workflow_hooks_command_runner_test.go |
Updates TryLock verification to include projectName argument. |
server/events/post_workflow_hooks_command_runner_test.go |
Updates TryLock verification to include projectName argument. |
server/events/mocks/mock_working_dir_locker.go |
Updates pegomock-generated mock/verifier to the new TryLock signature and captured args. |
| whPreWorkflowHookRunner.VerifyWasCalled(Never()).Run(Any[models.WorkflowHookCommandContext](), Eq(testHook.RunCommand), | ||
| Eq(defaultShell), Eq(defaultShellArgs), Eq(repoDir)) | ||
| preWhWorkingDirLocker.VerifyWasCalled(Never()).TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, "", command.Plan) | ||
| preWhWorkingDirLocker.VerifyWasCalled(Never()).TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, "", "", command.Plan) |
There was a problem hiding this comment.
This VerifyWasCalled(Never()).TryLock(...) uses concrete argument values, so it only verifies that TryLock was never called with this exact parameter set. If TryLock were called with different path/projectName values, this test would still pass. Consider using Any[...]() matchers for all parameters (or another pegomock pattern) to assert TryLock is never called at all in this branch.
| preWhWorkingDirLocker.VerifyWasCalled(Never()).TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, "", "", command.Plan) | |
| preWhWorkingDirLocker.VerifyWasCalled(Never()).TryLock( | |
| Any[string](), | |
| Any[int](), | |
| Any[string](), | |
| Any[string](), | |
| Any[string](), | |
| Any[command.Name](), | |
| ) |
There was a problem hiding this comment.
@copilot interesting point, but do we want to assert this was never called? Presumably we want some of these args to be concrete, and others Any?
| whPostWorkflowHookRunner.VerifyWasCalled(Never()).Run(Any[models.WorkflowHookCommandContext](), | ||
| Eq(testHook.RunCommand), Eq(defaultShell), Eq(defaultShellArgs), Eq(repoDir)) | ||
| postWhWorkingDirLocker.VerifyWasCalled(Never()).TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, "path", command.Plan) | ||
| postWhWorkingDirLocker.VerifyWasCalled(Never()).TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, "path", "", command.Plan) |
There was a problem hiding this comment.
This VerifyWasCalled(Never()).TryLock(...) uses concrete argument values, so it only verifies that TryLock was never called with this exact parameter set. If TryLock were called with different path/projectName values, this test would still pass. Consider using Any[...]() matchers for all parameters (or another pegomock pattern) to assert TryLock is never called at all in this branch.
| postWhWorkingDirLocker.VerifyWasCalled(Never()).TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, "path", "", command.Plan) | |
| postWhWorkingDirLocker.VerifyWasCalled(Never()).TryLock(Any[string](), Any[int](), Any[string](), Any[string](), Any[string](), Any[command.Name]()) |
what
Update mocks for change to working dir locker interface
why
#6086 changed the interface of TryLock, and it does appear that new mocks were generated, but somehow not all the mock was updated (maybe an older version of pegomock?)
Either way, I noticed this when running pegomock for an unrelated reason, and saw that the verify code in a unit test had the wrong number of args.
tests
Ran unit tests
references
Bug introduced in: #6086