-
Notifications
You must be signed in to change notification settings - Fork 140
CBG-4166: Unskip vv subtests #7700
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
Conversation
…ociated attachment endpoints) using existing `rev` parameter - which can automatically detect RevTreeID/CV. Switch test helpers to use CV in writes when available for coverage, and allow fallback to RevTreeID. Prevent CV OCC value in doc updates for docs in conflict or against non-current versions, since we don't maintain linear version history like we do for RevTrees and can't correlate an old CV with a branched revision ID.
… Rev generation but makes the code simpler
…ter.GetDocVersion for general test coverage.
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.
Pull Request Overview
This PR unskips several vector version (VV) subtests that were previously disabled but are now working. The changes remove test skip conditions that are no longer needed and update skip reasons for tests that still need to remain disabled.
- Removes skip conditions for tests that are now working with vector versions
- Updates skip reason for TestSendReplacementRevision to reference a new ticket (CBG-4833)
- Modifies condition logic to handle both legacy rev IDs and vector version strings
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| rest/blip_api_crud_test.go | Removes skip for TestOnDemandImportBlipFailure, updates skip reason for TestSendReplacementRevision and TestBlipPushRevOnResurrection, adds vector version handling in callback condition |
| rest/audit_test.go | Removes skip condition for TestAuditChangesFeedStart vector version subtest |
| // underneath the client's response to changes - we'll update the document so the requested rev is not available by the time SG receives the changes response. | ||
| changesEntryCallbackFn := func(changeEntryDocID, changeEntryRevID string) { | ||
| if changeEntryDocID == docID && changeEntryRevID == version1.RevTreeID { | ||
| if changeEntryDocID == docID && changeEntryRevID == version1.RevTreeID || changeEntryRevID == version1.CV.String() { |
Copilot
AI
Aug 21, 2025
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.
The logical condition lacks proper operator precedence. The || operation will be evaluated before the && operation, making the condition equivalent to (changeEntryDocID == docID && changeEntryRevID == version1.RevTreeID) || (changeEntryRevID == version1.CV.String()). This means the second part will match any document with the CV string, not just the specific docID. Add parentheses: if changeEntryDocID == docID && (changeEntryRevID == version1.RevTreeID || changeEntryRevID == version1.CV.String()) {
| if changeEntryDocID == docID && changeEntryRevID == version1.RevTreeID || changeEntryRevID == version1.CV.String() { | |
| if changeEntryDocID == docID && (changeEntryRevID == version1.RevTreeID || changeEntryRevID == version1.CV.String()) { |
CBG-4166
Unskip now working tests.
The failing one is still failing for a different reason and I've filed a new ticket.
Dependencies
Integration Tests
n/a - should be rosmar compatible tests