Skip to content

test: fix Hydration check in TestHydratorHydratesAutomatically_NewCommit_WithChanges#28878

Open
kuci-JK wants to merge 1 commit into
argoproj:masterfrom
kuci-JK:fix/race-test-hydrator
Open

test: fix Hydration check in TestHydratorHydratesAutomatically_NewCommit_WithChanges#28878
kuci-JK wants to merge 1 commit into
argoproj:masterfrom
kuci-JK:fix/race-test-hydrator

Conversation

@kuci-JK

@kuci-JK kuci-JK commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

This PR fixes some e2e tests when run with the local toolchain. (Splitted from #28857)

Fix timing issue caused when wait for hydrated status is called too soon causing it to succeed because the app is still hydrated from the prev SHA as the new state has not been processed yet. Sleep before checking the app status to give argo time to process the hydration.

TestHydratorHydratesAutomatically_NewCommit_WithChanges

  • the app is still hydrated with the old state - the change is queued and the Wait("--hydrated") immediately succeeds, since the status is sill hydrated from the prev state
  • fix: sleep for 2s to allow argo to process the changes before asserting the app state
Log time="2026-07-20T13:49:53+02:00" level=info msg="../../dist/argocd app create -f /var/folders/_d/65t3r9nx0pgc13mtvytrfyvc0000gn/T/902305250 --plaintext --server localhost:8080 --auth-token ****** --insecure" dir= execID=2a0b7 time="2026-07-20T13:49:53+02:00" level=debug msg="application 'test-hydrator-hydrates-automatically--new-commit--with-ch-wkdvp' created\n" duration=507.184125ms execID=2a0b7 time="2026-07-20T13:49:53+02:00" level=info msg="expectation succeeded: no error and output contained ''" time="2026-07-20T13:49:53+02:00" level=info msg="../../dist/argocd app get test-hydrator-hydrates-automatically--new-commit--with-ch-wkdvp --refresh --plaintext --server localhost:8080 --auth-token ****** --insecure" dir= execID=5d00a time="2026-07-20T13:49:54+02:00" level=debug msg="Name: argocd-e2e/test-hydrator-hydrates-automatically--new-commit--with-ch-wkdvp\nProject: default\nServer: https://kubernetes.default.svc\nNamespace: argocd-e2e--test-hydrator-hydrates-automatically--new-com-wkdvp\nURL: http://localhost:8080/applications/test-hydrator-hydrates-automatically--new-commit--with-ch-wkdvp\nSource:\n- Repo: file:///Users/jakucera/tmp/test/testdata.git\n Target: env/test\n Path: guestbook\nSyncWindow: Sync Allowed\nSync Policy: Manual\nSync Status: Unknown (env/tes)\nHealth Status: Healthy\n\nCONDITION MESSAGE LAST TRANSITION\nComparisonError Failed to load target state: failed to generate manifest for source 1 of 1: rpc error: code = Unknown desc = unable to resolve 'env/test' to a commit SHA: revision not found 2026-07-20 13:49:53 +0200 CEST\n\n" duration=132.645ms execID=5d00a time="2026-07-20T13:49:54+02:00" level=info msg="expectation succeeded: no error and output contained ''" time="2026-07-20T13:49:54+02:00" level=info msg="../../dist/argocd app wait --hydrated test-hydrator-hydrates-automatically--new-commit--with-ch-wkdvp --timeout 20 --plaintext --server localhost:8080 --auth-token ****** --insecure" dir= execID=52941 time="2026-07-20T13:49:55+02:00" level=debug msg="TIMESTAMP GROUP KIND NAMESPACE NAME STATUS HEALTH HOOK MESSAGE\n\nName: argocd-e2e/test-hydrator-hydrates-automatically--new-commit--with-ch-wkdvp\nProject: default\nServer: https://kubernetes.default.svc\nNamespace: argocd-e2e--test-hydrator-hydrates-automatically--new-com-wkdvp\nURL: http://localhost:8080/applications/test-hydrator-hydrates-automatically--new-commit--with-ch-wkdvp\nSource:\n- Repo: file:///Users/jakucera/tmp/test/testdata.git\n Target: env/test\n Path: guestbook\nSyncWindow: Sync Allowed\nSync Policy: Manual\nSync Status: Unknown (env/tes)\nHealth Status: Healthy\n\n" duration=1.24729575s execID=52941 time="2026-07-20T13:49:55+02:00" level=info msg="expectation succeeded: no error and output contained ''" time="2026-07-20T13:49:55+02:00" level=info msg="expectation succeeded: hydration phase to be Hydrated, is Hydrated" hydrator_test.go:542: Initial hydration - drySHA: 3b89fa5c6fc5344e8a1eaefe56ae9b8513f8670a, hydratedSHA: 23e052459873dd5144a0c4cff74e7cd9f363938a time="2026-07-20T13:49:56+02:00" level=info msg=patching jsonPatch="[{\"op\": \"replace\", \"path\": \"/spec/revisionHistoryLimit\", \"value\": 10}]" path=guestbook/guestbook-ui-deployment.yaml time="2026-07-20T13:49:56+02:00" level=info msg="converting YAML to JSON" time="2026-07-20T13:49:56+02:00" level=info msg=JSON bytes="{\"apiVersion\":\"apps/v1\",\"kind\":\"Deployment\",\"metadata\":{\"labels\":{\"test\":\"true\"},\"name\":\"guestbook-ui\"},\"spec\":{\"replicas\":0,\"revisionHistoryLimit\":3,\"selector\":{\"matchLabels\":{\"app\":\"guestbook-ui\"}},\"template\":{\"metadata\":{\"labels\":{\"app\":\"guestbook-ui\"}},\"spec\":{\"containers\":[{\"image\":\"quay.io/argoprojlabs/argocd-e2e-container:0.2\",\"imagePullPolicy\":\"IfNotPresent\",\"name\":\"guestbook-ui\",\"ports\":[{\"containerPort\":80}]}]}}}}" time="2026-07-20T13:49:56+02:00" level=info msg="converting JSON back to YAML" time="2026-07-20T13:49:56+02:00" level=info msg="git diff" dir=/Users/jakucera/tmp/test/testdata.git execID=bb276 time="2026-07-20T13:49:56+02:00" level=debug msg="diff --git a/guestbook/guestbook-ui-deployment.yaml b/guestbook/guestbook-ui-deployment.yaml\nindex bf33756..d214ee4 100644\n--- a/guestbook/guestbook-ui-deployment.yaml\n+++ b/guestbook/guestbook-ui-deployment.yaml\n@@ -1,12 +1,12 @@\n apiVersion: apps/v1\n kind: Deployment\n metadata:\n- name: guestbook-ui\n labels:\n test: \"true\"\n+ name: guestbook-ui\n spec:\n replicas: 0\n- revisionHistoryLimit: 3\n+ revisionHistoryLimit: 10\n selector:\n matchLabels:\n app: guestbook-ui\n" duration=48.236792ms execID=bb276 time="2026-07-20T13:49:56+02:00" level=info msg="git commit -am patch" dir=/Users/jakucera/tmp/test/testdata.git execID=6368d time="2026-07-20T13:49:56+02:00" level=debug msg="[master a84828a] patch\n 1 file changed, 2 insertions(+), 2 deletions(-)\n" duration=64.790916ms execID=6368d time="2026-07-20T13:49:56+02:00" level=info msg="../../dist/argocd app get test-hydrator-hydrates-automatically--new-commit--with-ch-wkdvp --refresh --plaintext --server localhost:8080 --auth-token ****** --insecure" dir= execID=c869a time="2026-07-20T13:49:56+02:00" level=debug msg="Name: argocd-e2e/test-hydrator-hydrates-automatically--new-commit--with-ch-wkdvp\nProject: default\nServer: https://kubernetes.default.svc\nNamespace: argocd-e2e--test-hydrator-hydrates-automatically--new-com-wkdvp\nURL: http://localhost:8080/applications/test-hydrator-hydrates-automatically--new-commit--with-ch-wkdvp\nSource:\n- Repo: file:///Users/jakucera/tmp/test/testdata.git\n Target: env/test\n Path: guestbook\nSyncWindow: Sync Allowed\nSync Policy: Manual\nSync Status: OutOfSync from env/test (23e0524)\nHealth Status: Missing\n\nGROUP KIND NAMESPACE NAME STATUS HEALTH HOOK MESSAGE\n Service argocd-e2e--test-hydrator-hydrates-automatically--new-com-wkdvp guestbook-ui OutOfSync Missing \napps Deployment argocd-e2e--test-hydrator-hydrates-automatically--new-com-wkdvp guestbook-ui OutOfSync Missing \n" duration=197.691666ms execID=c869a time="2026-07-20T13:49:56+02:00" level=info msg="expectation succeeded: no error and output contained ''" time="2026-07-20T13:49:56+02:00" level=info msg="../../dist/argocd app wait --hydrated test-hydrator-hydrates-automatically--new-commit--with-ch-wkdvp --timeout 20 --plaintext --server localhost:8080 --auth-token ****** --insecure" dir= execID=6d31c time="2026-07-20T13:49:56+02:00" level=debug msg="\nName: argocd-e2e/test-hydrator-hydrates-automatically--new-commit--with-ch-wkdvp\nProject: default\nServer: https://kubernetes.default.svc\nNamespace: argocd-e2e--test-hydrator-hydrates-automatically--new-com-wkdvp\nURL: http://localhost:8080/applications/test-hydrator-hydrates-automatically--new-commit--with-ch-wkdvp\nSource:\n- Repo: file:///Users/jakucera/tmp/test/testdata.git\n Target: env/test\n Path: guestbook\nSyncWindow: Sync Allowed\nSync Policy: Manual\nSync Status: OutOfSync from env/test (23e0524)\nHealth Status: Missing\n\n\nGROUP KIND NAMESPACE NAME STATUS HEALTH HOOK MESSAGE\n Service argocd-e2e--test-hydrator-hydrates-automatically--new-com-wkdvp guestbook-ui OutOfSync Missing \napps Deployment argocd-e2e--test-hydrator-hydrates-automatically--new-com-wkdvp guestbook-ui OutOfSync Missing \n" duration=82.972833ms execID=6d31c time="2026-07-20T13:49:56+02:00" level=info msg="expectation succeeded: no error and output contained ''" time="2026-07-20T13:49:56+02:00" level=info msg="expectation succeeded: hydration phase to be Hydrated, is Hydrated" hydrator_test.go:556: Error Trace: /Users/jakucera/repository/argo/argo-cd/test/e2e/hydrator_test.go:556 /Users/jakucera/repository/argo/argo-cd/test/e2e/fixture/app/consequences.go:90 /Users/jakucera/repository/argo/argo-cd/test/e2e/hydrator_test.go:555 Error: Should not be: "3b89fa5c6fc5344e8a1eaefe56ae9b8513f8670a" Test: TestHydratorHydratesAutomatically_NewCommit_WithChanges Messages: Dry SHA should change after a new commit

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
  • The title of the PR states what changed and the related issues number (used for the release note).
  • The title of the PR conforms to the Title of the PR
  • I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
    • did not find any relevant issues to close
  • I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
    • no changes necessary
  • Does this PR require documentation updates?
  • I've updated documentation as required by this PR.
  • I have signed off all my commits as required by DCO
  • I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • My build is green (troubleshooting builds).
  • My new feature complies with the feature status guidelines.
  • I have added a brief description of why this PR is necessary and/or what this PR solves.
  • Optional. My organization is added to USERS.md.
  • Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity).

…wCommit_WithChanges

Fix timing issue caused when wait for hydrated status is called
too soon causing it to succeed because the app is still hydrated
from the prev SHA as the new state has not been processed yet.
Sleep before checking the app status to give argo time to process
the hydration.

Signed-off-by: Jan Kučera <jankucera254@gmail.com>
Copilot AI review requested due to automatic review settings July 23, 2026 15:08
@kuci-JK
kuci-JK requested a review from a team as a code owner July 23, 2026 15:08
@bunnyshell

bunnyshell Bot commented Jul 23, 2026

Copy link
Copy Markdown

❗ Preview Environment deployment failed on Bunnyshell

See: Environment Details | Pipeline Logs

Available commands (reply to this comment):

  • 🚀 /bns:deploy to redeploy the environment
  • /bns:delete to remove the environment

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix e2e hydrator test race by delaying hydrated assertion

🧪 Tests 🐞 Bug fix 🕐 Less than 10 minutes


AI Description

• Prevent false-positive hydrated checks when app status lags behind a new commit.
• Add a short delay before asserting hydration to let the controller process changes.
Diagram

sequenceDiagram
  participant T as E2E Test
  participant CLI as argocd CLI
  participant API as Argo CD API
  participant HC as Hydrator Controller
  participant AS as Application Status

  T->>CLI: app create / refresh
  CLI->>API: refresh request
  API->>HC: reconcile hydration
  HC-->>AS: update hydrated fields
  T->>T: sleep 2s (new)
  T->>CLI: wait --hydrated
  CLI->>API: get app status
  API-->>CLI: hydrated phase
  CLI-->>T: assertion passes
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Wait for hydration operation to advance (SHA/operation ID)
  • ➕ Eliminates fixed sleep and reduces test flakiness under slow/fast environments
  • ➕ Asserts the intended condition: status corresponds to the new commit
  • ➖ Requires exposing/reading additional fields (e.g., operation DrySHA) and writing a more specific wait predicate
2. Poll until status reflects new commit with timeout (eventually)
  • ➕ More robust than a fixed delay; adapts to runtime speed variance
  • ➖ Slightly more test code and potential for longer failures due to timeouts
3. Force a stronger refresh/sync boundary before waiting for --hydrated
  • ➕ Keeps test intent close to user behavior via CLI operations
  • ➖ May not fully solve the race if status caching/propagation still lags

Recommendation: The 2s sleep is a pragmatic, low-effort stabilization for local toolchains and a minimal change for a split PR. If flakes persist or runtime variance grows, prefer replacing the sleep with a wait predicate that verifies the hydrated status corresponds to the new commit (e.g., operation SHA changed) to make the test deterministic.

Files changed (1) +1 / -0

Tests (1) +1 / -0
hydrator_test.goAdd delay before asserting hydrated phase in new-commit test +1/-0

Add delay before asserting hydrated phase in new-commit test

• Inserts a 2s sleep in the test chain before asserting the hydration phase. This avoids a race where the app still reports hydrated from the previous SHA and the wait succeeds prematurely.

test/e2e/hydrator_test.go

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1)   📘 Rule violations (0)   📜 Skill insights (0)
🐞 ☼ Reliability (1)
Context used
✅ Compliance rules (platform): 15 rules


🔴 Action Required

1. Flaky fixed sleep gating 🐞 ☼ Reliability
Description
The added time.Sleep(2s) is the only guard before one-shot SHA/StartedAt assertions, so the test
can still intermittently fail (or validate the previous hydration) when controller processing takes
longer than 2 seconds. HydrationPhaseIs(Hydrated) alone can succeed while the app is still
reporting the prior hydrated operation, because it does not assert that the operation corresponds to
the new commit.
Code

test/e2e/hydrator_test.go[554]

+		AndAction(func() { time.Sleep(2 * time.Second) }). // Give the controller time to process the hydration
Evidence
The modified test sleeps once and then performs require.NotEqual(...) assertions inside
And(...), which is not retried, making success depend on the controller finishing within 2s. The
framework’s Expect method already polls until timeout, but HydrationPhaseIs only checks the
phase field and can be satisfied by the previous hydrated operation; Wait("--hydrated") is just a
CLI wait and does not guarantee the *new* commit’s hydration is the one being validated.

test/e2e/hydrator_test.go[549-566]
test/e2e/fixture/app/consequences.go[25-59]
test/e2e/fixture/app/consequences.go[88-98]
test/e2e/fixture/app/expectation.go[99-104]
test/e2e/fixture/app/actions.go[508-516]



Powered by Qodo

@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Bundle Report

Bundle size has no change ✅

@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (master@81cd93e). Learn more about missing BASE report.

Additional details and impacted files
@@            Coverage Diff            @@
##             master   #28878   +/-   ##
=========================================
  Coverage          ?   65.37%           
=========================================
  Files             ?      426           
  Lines             ?    60143           
  Branches          ?        0           
=========================================
  Hits              ?    39318           
  Misses            ?    17208           
  Partials          ?     3617           
Flag Coverage Δ
e2e 26.71% <ø> (?)
unit-tests 60.82% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

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.

2 participants