Skip to content

Fix flaky NPCM7xx timer test and enable PR CI#17

Merged
zevorn merged 6 commits into
processmission:develfrom
zevorn:codex/fix-npcm7xx-periodic-timer-test
Jul 20, 2026
Merged

Fix flaky NPCM7xx timer test and enable PR CI#17
zevorn merged 6 commits into
processmission:develfrom
zevorn:codex/fix-npcm7xx-periodic-timer-test

Conversation

@zevorn

@zevorn zevorn commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • advance NPCM7xx periodic timer qtests by the configured timer period
  • run CI for pull requests targeting devel and pushes to devel
  • derive checkpatch ranges directly from pull request or push event metadata
  • report checkpatch warnings without failing CI; keep errors fatal

Root cause

clock_step_next() advances to the earliest pending timer in the whole machine. An unrelated timer can therefore stop virtual time before the NPCM7xx timer under test expires, leaving TISR clear and the IRQ deasserted.

The Actions workflow also listened only for push, so fork pull requests were not tested in processmission/qemu before merge. Its checkpatch job could write an empty range and silently skip when no same-repository PR was found.

CI behavior

Pull requests targeting devel now use the PR base and head metadata to compute an exact checkpatch range. Pushes to devel use the before/current SHA range. Missing metadata fails the range step instead of skipping checkpatch.

Checkpatch runs with explicit --no-strict: warning-only results return success, while errors remain fatal.

Testing

  • built qemu-system-arm and tests/qtest/npcm7xx_timer-test
  • qemu:qtest-arm/npcm7xx_timer-test: 180 subtests passed
  • repeated the complete qtest 100 times: 100/100 passed
  • warning-only checkpatch fixture: exit 0
  • error checkpatch fixture: exit 1
  • scripts/checkpatch.pl --no-strict origin/devel..HEAD: clean
  • actionlint -shellcheck=: clean
  • git diff --check: clean

@zevorn
zevorn marked this pull request as ready for review July 20, 2026 05:20
clock_step_next() advances to the next timer in the whole machine. An
unrelated timer can therefore stop the clock before the NPCM7xx timer
expires, leaving TISR clear and the IRQ deasserted.

Advance the virtual clock by the configured NPCM7xx timer period in both
periodic tests so each iteration reaches the expiration being checked.

Signed-off-by: Chao Liu <chao.liu@processmission.com>
@zevorn
zevorn force-pushed the codex/fix-npcm7xx-periodic-timer-test branch from 131ea9f to 310079d Compare July 20, 2026 05:22
The workflow currently only listens for push events. Pull requests from
forks therefore are not tested in processmission/qemu before merge.

Run CI for pull requests targeting devel and keep push coverage on devel.
Compute checkpatch ranges directly from the event metadata so the job
cannot silently skip. Pass --no-strict explicitly so warnings remain
informational while errors still fail the job.

Signed-off-by: Chao Liu <chao.liu@processmission.com>
@zevorn zevorn changed the title tests/qtest: fix flaky NPCM7xx periodic timer test Fix flaky NPCM7xx timer test and enable PR CI Jul 20, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f0f2a2c937

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/build.yml Outdated
Restricting push events to devel prevents p-b-o/qemu-ci from running
build and checkpatch jobs for mailing-list series branches.

Restore the original all-branch push coverage while keeping pull request
runs limited to changes targeting devel.

Signed-off-by: Chao Liu <chao.liu@processmission.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e01ecceedc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/build.yml Outdated
zevorn added 2 commits July 20, 2026 14:35
GitHub reports forty zeroes as the before SHA when a push creates a
branch. Passing that sentinel to git merge-base makes checkpatch fail
before it can inspect the series.

Fetch the repository default branch and use its merge base with the new
branch tip for that event. Keep the existing before-to-after range for
ordinary pushes.

Signed-off-by: Chao Liu <chao.liu@processmission.com>
Several build jobs enable documentation implicitly when the container
provides Sphinx. These duplicate builds add no compiler or target
coverage and have repeatedly failed when Sphinx parallel workers exit
with EOFError.

Disable documentation through one shared option for every ordinary build.
Keep documentation enabled in build-misc, run its Sphinx target first, and
force Python to report one CPU so its -j auto mode stays serial.

Signed-off-by: Chao Liu <chao.liu@processmission.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f941a61294

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/build.yml
The VNC listen-change test allocates all three ports before launching
QEMU. Ports.find_free_ports() only probes and releases each port, so the
unused ports can be claimed while the initial VNC listener starts.

Allocate only the initial listener port up front. Select the replacement
ports after QEMU is running and use them immediately for display-update,
which keeps the unreserved-port window short.

Signed-off-by: Chao Liu <chao.liu@processmission.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7eb2a6c062

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

fi

git fetch --no-tags origin "$DEFAULT_BRANCH"
base="$(git merge-base "origin/$DEFAULT_BRANCH" "$CURRENT_SHA")"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Skip checkpatch for an empty new-branch range

When a branch is first pushed at exactly the default branch tip, PUSH_BEFORE_SHA is zero and this merge base equals CURRENT_SHA, producing a range such as abc..abc. scripts/checkpatch.pl rejects that empty revision list with “no revisions returned” and exits 255, so merely creating a branch causes the checkpatch job to fail despite there being no patch to inspect. The fresh sentinel-handling path therefore needs to detect and skip an empty range.

Useful? React with 👍 / 👎.

@zevorn
zevorn merged commit dbfbf23 into processmission:devel Jul 20, 2026
44 checks passed
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.

1 participant