Skip to content

Conversation

@psnoch-akamai
Copy link
Contributor

✔️ How to Test

make TEST_ARGS="-run TestTryToLockTwoResourcesWithTheSameType"
make TEST_ARGS="-run TestTryToCreateWithInvalidData"

Copilot AI review requested due to automatic review settings December 18, 2025 15:37
@psnoch-akamai psnoch-akamai requested a review from a team as a code owner December 18, 2025 15:37
@psnoch-akamai psnoch-akamai requested review from ezilber-akamai and vshanthe and removed request for a team December 18, 2025 15:37
Copy link
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 adds two new integration tests for the Resource lock functionality to improve test coverage. The tests verify error handling for edge cases: attempting to create multiple locks of different types on the same resource, and attempting to create a lock with an invalid entity ID.

Key Changes

  • Added TestTryToLockTwoResourcesWithTheSameType to verify that conflicting lock types cannot be applied simultaneously to the same resource
  • Added TestTryToCreateWithInvalidData to verify proper error handling when creating locks with non-existent entity IDs
  • Created supporting test fixtures with recorded HTTP interactions for both test scenarios

Reviewed changes

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

File Description
test/integration/locks_test.go Added two new test functions with error validation for lock creation edge cases
test/integration/fixtures/TestLockTryToLockResourceTwice.yaml HTTP interaction recording for the duplicate lock type test scenario
test/integration/fixtures/TestLockTryToCreateWithNotExistingEntityID.yaml HTTP interaction recording for the invalid entity ID test scenario

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +97 to +98
_, errAlreadyExist := client.CreateLock(context.Background(), createOpts)
require.Error(t, errAlreadyExist)
Copy link

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

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

The variable name errAlreadyExist is misleading. The test verifies that conflicting lock types cannot coexist, not that a duplicate lock already exists. Consider renaming to errConflictingLock or err to better reflect the actual validation being performed.

Suggested change
_, errAlreadyExist := client.CreateLock(context.Background(), createOpts)
require.Error(t, errAlreadyExist)
_, errConflictingLock := client.CreateLock(context.Background(), createOpts)
require.Error(t, errConflictingLock)

Copilot uses AI. Check for mistakes.
@psnoch-akamai psnoch-akamai deleted the resource-lock-extra-integration-tests branch December 18, 2025 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants