Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .github/workflows/juno-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,16 @@ concurrency:

jobs:
test:
name: Run Tests
strategy:
fail-fast: false
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 }}
Comment on lines 18 to +25

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.

steps:
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
- name: Set up go
Expand Down Expand Up @@ -69,3 +71,4 @@ jobs:
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.

Loading