[compot-dev]

The check that QE is not a protection on a Macronix chip looked only for the word PROTECTED in the output of -t, so a self-test that failed for another reason passed it. It now goes through t 0, which checks the exit code: with the erase failing it reports FAIL.

Tests only: no new version, the release files do not change.

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Test Macronix QE self-test by exit code

🧪 Tests 🐞 Bug fix 🕐 Less than 5 minutes

AI Description

• Replace the Macronix QE output check with an expected exit-code check.
• Ensure the test fails when the self-test fails for an unrelated reason.

Diagram

graph TD
  A["Macronix QE case"] --> B["CLI self-test"] --> C["Exit-code helper"] --> D["Test verdict"]

Loading

High-Level Assessment

Using the existing exit-code helper matches adjacent self-test cases and catches failures that an output-only check misses. Adding a separate exit-code check alongside grep would duplicate the helper's behavior without improving this assertion.

Files changed (1) +1 / -1

Tests (1) +1 / -1

run.shAssert successful exit for the Macronix QE self-test +1/-1

Assert successful exit for the Macronix QE self-test

• Replaces a check for the absence of 'PROTECTED' in '-t' output with 't 0'. The case now fails if the self-test exits unsuccessfully for any reason, while retaining the same Macronix register scenario.

tests/run.sh

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Erase-failure case is never tested ✓ Resolved

Description

The rewritten case on line 76 runs the Macronix QE self-test without injecting a failure, so it only
checks that an unprotected block exits 0. The stub's erase-failure switch is never set, so the
failing-erase path the PR claims to fix goes untested, and the same case would also pass on the old
code.

---

2. Test change merges without running CI 🐞 Bug ☼ Reliability

Description

The commit message and PR description carry the skip-CI marker, so this edit to the test script
merges without the suite running on it. The new assertion stays unverified until a later change runs
the suite.

Code

tests/run.sh[76]

+t 0 "-t Macronix: QE is not a protection, ff and 07 are not read as registers" MS_MACRONIX=1 MS_SR=40ff07 $B -t

Evidence

The PR is a test-only change meant to make an assertion stricter, and skipping CI means its result
is not verified before merge. The merge is also tied to an unrelated change that the description
does not describe, and the repository's CI config contains no skip rule, so the commit marker is
what bypasses the run.

tests/run.sh[76-76]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR asks for CI to be skipped, so the rewritten Macronix QE assertion in tests/run.sh is merged without the test suite running on it.
## Fix Focus Areas
- tests/run.sh[76-76]
## Recommended Fix
Remove the skip-CI marker from the commit and merge only after the suite passes on this exact change, or combine it with the other change and let CI run on the combined commit.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can commit Qodo's fix in one click with committable suggestions (GitHub & GitLab)

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[compot-dev]

/review

[qodo-free-for-open-source-projects]

Code review by qodo was updated up to the latest commit 90e1f76

[compot-dev]

The second Qodo finding no longer applies: Build and Version ran on the final head 90e1f76 and passed. The first finding is fixed in 90e1f76 by the MS_FAIL=erase case, which expects exit 1.