Skip to content

Added delta-sync history based tests#416

Open
vipbhardwaj wants to merge 10 commits into
mainfrom
CBG-5106
Open

Added delta-sync history based tests#416
vipbhardwaj wants to merge 10 commits into
mainfrom
CBG-5106

Conversation

@vipbhardwaj
Copy link
Copy Markdown
Contributor

@vipbhardwaj vipbhardwaj commented May 20, 2026

CBG-5388

Summary

  • Add a new QE test file tests/QE/test_replication_upgrade_delta_sync.py covering delta-sync history behavior on upgrade flows, including:
    • test_delta_sync_history_pull_post_upgrade_sgw_mutation — pull of a 4.x SGW mutation (revtree + fresh HLV) into a client holding a revtree-only ancestor. Marked xfail(strict=True) to track the open SGW delta-sync history bug where the rev message's history field comes back empty.
    • test_delta_sync_history_pull_pre_upgrade_sgw_two_revs — pull of a revtree-only rev that exists on both sides, verifying the local side resolves to an RTE-encoded HLV while SGW's HLV remains absent.
  • Extract the shared upgrade-test scaffolding out of tests/dev_e2e/test_replication_upgrade.py into a new reusable helper module client/src/cbltest/api/upgrade_test_helpers.py:
    • setup_upgrade_env(...) — version/dataset skip checks, SG upgrade DB teardown, CBS bucket restore, local DB reset.
    • do_upgrade_replication_test(...) — replicator setup, optional doc-event waiting, pre/post snapshot logging, optional compare_local_and_remote, and an injectable DocValidator.
    • DocSnapshot / DocValidator exposed as module-level types.
  • Refactor tests/dev_e2e/test_replication_upgrade.py to consume the shared helpers — pure de-duplication, no behavior change for the existing nonconflict_* cases.

Motivation

The existing TestReplicationUpgrade class duplicated all of its environment-setup and replicator-driver logic inside the test class itself. The new delta-sync history tests need exactly the same scaffolding (4.0 dataset, upgrade SG DB, revid/HLV snapshots, custom validators), but with a different SG database config (delta_sync enabled) and an explicit SGW write step before replication. Promoting the helpers to cbltest.api.upgrade_test_helpers lets both suites share the harness without copy-paste, and unblocks adding more upgrade-history scenarios.

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

Adds new upgrade-flow replication tests focused on delta-sync history behavior, and refactors the existing upgrade replication suite to share a common harness.

Changes:

  • Added tests/QE/test_replication_upgrade_delta_sync.py to cover delta-sync history scenarios (including an xfail tracking an SGW bug).
  • Introduced client/src/cbltest/api/upgrade_test_helpers.py to centralize upgrade test environment setup + replication driver logic.
  • Refactored tests/dev_e2e/test_replication_upgrade.py to use the shared helpers (de-duplication).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
tests/QE/test_replication_upgrade_delta_sync.py New QE tests for delta-sync history behavior during upgrade-related replication flows.
client/src/cbltest/api/upgrade_test_helpers.py New shared helper module for upgrade env setup and replication execution/validation.
tests/dev_e2e/test_replication_upgrade.py Updated to use the shared helper functions/types instead of local duplicated scaffolding.

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

Comment thread tests/QE/test_replication_upgrade_delta_sync.py
Comment thread client/src/cbltest/api/upgrade_test_helpers.py Outdated
Comment thread client/src/cbltest/api/upgrade_test_helpers.py Outdated
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

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

Comment thread tests/QE/test_replication_upgrade_delta_sync.py Outdated
Comment thread tests/QE/test_replication_upgrade_delta_sync.py Outdated
Comment thread tests/shared/upgrade_test_helpers.py
Comment thread tests/dev_e2e/test_replication_upgrade.py Outdated
Comment thread tests/dev_e2e/test_replication_upgrade.py Outdated
Comment thread tests/dev_e2e/test_replication_upgrade.py Outdated
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

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

Comment thread client/src/cbltest/api/syncgateway.py
@vipbhardwaj
Copy link
Copy Markdown
Contributor Author

Resolved all the comments from co-pilot.
@borrrden Any input for TDK..?
@bbrks @gregns1 anything wrongly written from the way you wanted the test to go?

@vipbhardwaj vipbhardwaj requested review from bbrks, borrrden and gregns1 May 22, 2026 06:43
Comment thread tests/shared/upgrade_test_helpers.py
Copy link
Copy Markdown
Member

@borrrden borrrden left a comment

Choose a reason for hiding this comment

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

For TDK API of course this is fine since now very little has changed 😄

Copy link
Copy Markdown

@gregns1 gregns1 left a comment

Choose a reason for hiding this comment

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

Looks good from my end too. SGW fix for this will be coming next week.

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

Copilot reviewed 6 out of 7 changed files in this pull request and generated 8 comments.

Comment thread tests/shared/upgrade_test_helpers.py
Comment thread tests/shared/upgrade_test_helpers.py
Comment thread tests/QE/test_replication_upgrade_delta_sync.py Outdated
Comment thread tests/QE/test_replication_upgrade_delta_sync.py Outdated
Comment thread tests/QE/test_replication_upgrade_delta_sync.py
Comment thread spec/tests/QE/test_replication_upgrade_delta_sync.md Outdated
Comment thread spec/tests/QE/test_replication_upgrade_delta_sync.md Outdated
Comment thread client/src/cbltest/api/syncgateway.py
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown

@gregns1 gregns1 left a comment

Choose a reason for hiding this comment

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

Did some work on getting trace logs for the above this morning, test case 2 (the test using doc ID nonconflict_2) is not actually sending the rev as a delta as it stands. This is because the backup revision for rev 1 is not present. I think this is because of the way the test is setup which is pre seeding the bucket with legacy revs and the backup revs may not be getting seeded with the legacy revisions. For this particular test case to work as intended we need to be able to have a backup revision in the bucket for rev 1 for SGW to calculate a delta from.

2026-05-26T11:27:30.317+01:00 [DBG] CRUD+: c:[17f680c5] db:upgrade col:_default No old revision "<ud>nonconflict_2</ud>" / "1-e643a00fb9cad8cf87ee4d2c4e2383f6ecf118c0"
2026-05-26T11:27:30.317+01:00 [DBG] Sync+: c:[17f680c5] db:upgrade col:_default Falling back to full body replication. Couldn't get delta from 1-e643a00fb9cad8cf87ee4d2c4e2383f6ecf118c0 to 2-36a2cfe8544956ad3c5b6491cbfc7d94 for key <ud>nonconflict_2</ud> - err: 404 missing
....
2026-05-26T11:27:30.317+01:00 [TRC] Sync+: c:[17f680c5] db:upgrade col:_default Sending revision <ud>nonconflict_2</ud>/2-36a2cfe8544956ad3c5b6491cbfc7d94, body:<ud>{"channels":["numbers"],"number":2}</ud>, properties: <ud>map[history:1-e643a00fb9cad8cf87ee4d2c4e2383f6ecf118c0 sequence:6]</ud>, attDigests: [] -- db.(*BlipSyncContext).sendRevisionWithProperties() at blip_sync_context.go:456

As you can see above its falling back to send the revision as a full revision in this case so the delta sync code pathway isn't being executed properly in the test.
I'm not sure what the best way to solve this is in the TDK, we may need to look into how the revs are being populated in the bucket.

As for test case 1 (test using nonconflict_3 doc) I can see the revision is being sent as delta and the history is in fact empty:

2026-05-26T11:25:26.923+01:00 [TRC] Sync+: c:[51984a3e] db:upgrade col:_default docID: <ud>nonconflict_3</ud> - delta: <ud>{"updated_by":"delta_sync_history_test"}</ud> -- db.(*BlipSyncContext).sendRevAsDelta() at blip_handler.go:976
2026-05-26T11:25:26.923+01:00 [DBG] Sync+: c:[51984a3e] db:upgrade col:_default Sending rev "<ud>nonconflict_3</ud>" 18b316999d570000@q9I8PBb+Xkq2c5XWHt+rIA as delta. DeltaSrc:2-509dc0903869f6b103fd1266b169e73876b80d27
2026-05-26T11:25:26.923+01:00 [TRC] Sync+: c:[51984a3e] db:upgrade col:_default Sending revision <ud>nonconflict_3</ud>/18b316999d570000@q9I8PBb+Xkq2c5XWHt+rIA, body:<ud>{"updated_by":"delta_sync_history_test"}</ud>, properties: <ud>map[Content-Type:application/json Profile:rev collection:0 deltaSrc:2-509dc0903869f6b103fd1266b169e73876b80d27 history: id:nonconflict_3 rev:18b316999d570000@q9I8PBb+Xkq2c5XWHt+rIA sequence:34]</ud> -- db.(*BlipSyncContext).sendBLIPMessage() at blip_sync_context.go:626

I am guessing that because we send the delta source ID CBL is using that to identify its not a conflict, I will need to check up on this. Either way I can see the history is empty and it shouldn't be so I can get a fix together for that soon.

@vipbhardwaj vipbhardwaj requested a review from gregns1 May 27, 2026 10:54
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.

4 participants