Skip to content

New state: integrate new state tests into CI#3043

Open
brbrr wants to merge 7 commits into
mainfrom
add/ci-test-new-state
Open

New state: integrate new state tests into CI#3043
brbrr wants to merge 7 commits into
mainfrom
add/ci-test-new-state

Conversation

@brbrr
Copy link
Copy Markdown
Contributor

@brbrr brbrr commented Aug 7, 2025

No description provided.

@brbrr brbrr self-assigned this Aug 7, 2025
@brbrr brbrr changed the base branch from main to maksym/integrate-common-interfaces August 7, 2025 13:58
@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 7, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.49%. Comparing base (d29fc03) to head (09d6758).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3043      +/-   ##
==========================================
+ Coverage   76.47%   76.49%   +0.01%     
==========================================
  Files         402      402              
  Lines       36770    36770              
==========================================
+ Hits        28120    28127       +7     
+ Misses       6673     6666       -7     
  Partials     1977     1977              
Flag Coverage Δ
new-state 75.81% <ø> (?)
old-state 76.47% <ø> (?)

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

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Oct 8, 2025

This pull request is stale because it has been open 35 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions Bot added the Stale label Oct 8, 2025
@MaksymMalicki MaksymMalicki changed the base branch from maksym/integrate-common-interfaces to maksym/integrate-common-interface-rawdb October 18, 2025 14:32
@github-actions github-actions Bot removed the Stale label Oct 19, 2025
@github-actions
Copy link
Copy Markdown
Contributor

This pull request is stale because it has been open 35 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions Bot added Stale and removed Stale labels Dec 19, 2025
@MaksymMalicki MaksymMalicki force-pushed the maksym/integrate-common-interface-rawdb branch from 33d81d5 to dc23482 Compare December 29, 2025 15:11
@MaksymMalicki MaksymMalicki force-pushed the maksym/integrate-common-interface-rawdb branch 2 times, most recently from 374fcf7 to 1ba0bf0 Compare February 10, 2026 21:48
@MaksymMalicki MaksymMalicki force-pushed the maksym/integrate-common-interface-rawdb branch 4 times, most recently from b95e87a to 514d9fa Compare February 25, 2026 21:15
@MaksymMalicki MaksymMalicki force-pushed the maksym/integrate-common-interface-rawdb branch 2 times, most recently from fcdfbab to 86a5199 Compare March 24, 2026 17:17
@MaksymMalicki MaksymMalicki force-pushed the maksym/integrate-common-interface-rawdb branch 2 times, most recently from d42bd9c to 4e4d276 Compare April 11, 2026 23:52
@MaksymMalicki MaksymMalicki force-pushed the maksym/integrate-common-interface-rawdb branch 3 times, most recently from efa09d5 to 5bc5cd6 Compare April 18, 2026 15:15
@MaksymMalicki MaksymMalicki force-pushed the maksym/integrate-common-interface-rawdb branch 2 times, most recently from dd87a16 to 83e4759 Compare April 27, 2026 11:39
Base automatically changed from maksym/integrate-common-interface-rawdb to main April 28, 2026 11:38
@brbrr brbrr force-pushed the add/ci-test-new-state branch from ed14df4 to d6ad287 Compare May 9, 2026 16:24
@brbrr brbrr temporarily deployed to Development May 9, 2026 16:34 — with GitHub Actions Inactive
@brbrr brbrr temporarily deployed to Development May 11, 2026 07:04 — with GitHub Actions Inactive
@brbrr brbrr force-pushed the add/ci-test-new-state branch from 21084c8 to 86579be Compare May 11, 2026 10:18
Signed-off-by: Yaroslav Kukharuk <i.kukharuk@gmail.com>
@brbrr brbrr temporarily deployed to Development May 22, 2026 11:42 — with GitHub Actions Inactive
@brbrr brbrr marked this pull request as ready for review May 22, 2026 12:53
Copilot AI review requested due to automatic review settings May 22, 2026 12:53
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

Note

Copilot was unable to run its full agentic suite in this review.

Integrates the “new state” test mode into CI by expanding the existing test job matrix to run both old and new state variants and tag Codecov uploads accordingly.

Changes:

  • Add a juno-new-state boolean axis to the workflow matrix.
  • Plumb the matrix value into JUNO_NEW_STATE and job naming for visibility.
  • Add a Codecov flags value to distinguish old vs new state coverage uploads.

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

Comment thread .github/workflows/juno-test.yml Outdated
Comment thread .github/workflows/juno-test.yml Outdated
@brbrr brbrr temporarily deployed to Development May 22, 2026 14:28 — with GitHub Actions Inactive
@brbrr brbrr temporarily deployed to Development May 25, 2026 19:40 — with GitHub Actions Inactive
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 1, 2026

Claude encountered an error —— View job


I'll analyze this and get back to you.

@brbrr brbrr temporarily deployed to Development June 1, 2026 10:16 — with GitHub Actions Inactive
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 1, 2026

Claude encountered an error —— View job


I'll analyze this and get back to you.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 1, 2026

Claude finished @brbrr's task in 3m 4s —— View job


PR Review: Integrate new state tests into CI

  • Read the workflow file and PR diff
  • Verify Copilot's concern about matrix.juno-new-state (already fixed in commit 1c63f0ba — keys use underscores)
  • Cross-check Makefile (test, test-cover) against the new matrix axis and env var plumbing
  • Post inline findings on the workflow file

Summary

Important

  • On ubuntu-latest, the matrix axis is partly redundant and partly incorrect. make test-cover already runs both modes sequentially, and its first go test line does not clear JUNO_NEW_STATE — so when matrix.juno_new_state=true propagates through env:, the file written to coverage.old.out is actually produced under new-state mode. Net effect: ubuntu-latest runs the full coverage twice (2× wall time) and the per-flag separation in Codecov is muddled.
  • Both matrix variants upload the same two files (coverage.old.out,coverage.new.out) and differ only in the flags value, so the new-state / old-state flag split on Codecov isn't actually a clean separation of modes.

See inline comments on .github/workflows/juno-test.yml for two suggested fixes (drop the axis on ubuntu-latest, or refactor make test-cover to honor JUNO_NEW_STATE and produce one file per pass).

Looks good

  • The recent rename to juno_new_state (underscores) addresses Copilot's earlier concern.
  • The ${{ matrix.juno_new_state && ... || ... }} expressions for the job name and the Codecov flag evaluate correctly given GH Actions' truthy fallback semantics.
  • For non-ubuntu runners, make test does respect the inherited JUNO_NEW_STATE, so those matrix cells do exercise the new-state code path as intended.

Nits

token: ${{ secrets.CODECOV_TOKEN }}
fail_ci_if_error: true
files: coverage/coverage.old.out,coverage/coverage.new.out
flags: ${{ matrix.juno_new_state && 'new-state' || 'old-state' }}
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.

Both matrix variants upload the same two filesfiles: coverage.old.out,coverage.new.out on line 73 — differing only by the flags value here. Combined with the env-leak issue noted on the env block above, the new-state flag in Codecov ends up labelling the same artifact pair the old-state flag also labels, so per-flag coverage on Codecov is not actually measuring the two modes separately.

Aligned with the suggestion above: upload one file per matrix run (e.g., coverage.new.out when juno_new_state is true, otherwise coverage.old.out), or drop the matrix axis on ubuntu-latest and do two uploads from a single job — one per flag — pointing at the file actually produced by that mode.

Comment on lines 18 to +25
matrix:
os: [ubuntu-latest, macos-latest, ubuntu-arm64-4-core]
juno_new_state: [false, true]
runs-on: ${{ matrix.os }}
name: Run Tests${{ matrix.juno_new_state && ' [New State]' || '' }}
env:
VM_DEBUG: true
JUNO_NEW_STATE: ${{ matrix.juno_new_state }}
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.

Important — matrix axis is redundant and corrupts coverage on ubuntu-latest.

On ubuntu-latest the test step runs make test-cover, which already executes both modes back-to-back:

test-cover: clean-testcache rustdeps
    mkdir -p coverage
    go test $(GO_TAGS) -coverpkg=$(PKG) -coverprofile=coverage/coverage.old.out -covermode=atomic $(PKG)
    JUNO_NEW_STATE=true go test $(GO_TAGS) -coverpkg=$(PKG) -coverprofile=coverage/coverage.new.out -covermode=atomic $(PKG)

Two problems flow from this:

  1. Double work. With juno_new_state: [false, true], ubuntu-latest now runs the full coverage matrix twice — 4 full test passes instead of 2 — for the same artifacts.
  2. Env leak corrupts coverage.old.out when matrix.juno_new_state=true. The first line of test-cover does not unset JUNO_NEW_STATE, so it inherits true from the job env. The "old" coverage file then actually contains new-state coverage. The flags: new-state upload below will therefore include genuine new-state data, but the flags: old-state upload contains the correct mix — and both runs still upload the same two files, so the codecov flag split is mostly cosmetic.

Suggested fix — pick one:

  • (a) Restrict the matrix axis to non-ubuntu runners (make test honors JUNO_NEW_STATE correctly), and keep make test-cover running once on ubuntu-latest with one Codecov upload per file/flag.
  • (b) Refactor make test-cover to honor JUNO_NEW_STATE (single pass producing one file), then the matrix axis drives both modes uniformly and each upload carries the right flag.

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