Skip to content

ci: run the build-and-test workflow on master - #39

Open
srpatcha wants to merge 1 commit into
masterfrom
fix/ci-runs-on-master
Open

ci: run the build-and-test workflow on master#39
srpatcha wants to merge 1 commit into
masterfrom
fix/ci-runs-on-master

Conversation

@srpatcha

Copy link
Copy Markdown
Member

Closes #38.

ci.yml watched main and develop. Neither exists:

$ gh api repos/embeddedos-org/eAI -q .default_branch
master

$ gh api repos/embeddedos-org/eAI/branches/main
404 Branch not found

Every push to master and every pull request against it falls outside the
trigger. The last run was 2026-05-31, and the history shows what happened:

2026-05-31  branch=v1.9.0  failure
2026-05-28  branch=v1.5.0  failure
2026-05-27  branch=main    failure
2026-05-27  branch=main    success

main existed until late May, the workflow ran on it, the branch was renamed to
master, and the workflow was left pointing at a branch that had gone. The runs
after that are tag pushes, which still match tags: ["v*"] — and all three
failed.

What this does and does not claim

ci.yml is the only workflow here that compiles the C, runs pytest, and
cross-builds for ARM:

jobs:
  test-c:
  test-python:
  build-arm:
  release:

Five other workflows do watch master and have kept running — codeql,
book-build, video-build, deploy-pages, scorecard — so the repository has
not been silent, and I overstated that in the issue before correcting it.
codeql even has a build step, but as input to static analysis rather than as a
test gate.

The accurate statement: code-quality and docs pipelines are alive, and the
build-and-test pipeline has been dead since May.

The change

master is added rather than substituted, on both push and
pull_request, so a rename in either direction does not break this again. YAML
validated.

on:
  push:
    branches: [master, main, develop]
    tags: ["v*"]
  pull_request:
    branches: [master, main]

Expect the first run to be red

The last three runs failed and three months of changes have landed unverified
since. That is the point of turning it back on, and it should not be read as a
regression from this PR.

Not confined to this repository

Same pattern in five others — default branch master, ci.yml watching
[main, develop]:

repo ci.yml watches build/test steps
eAI [main, develop] 6
eNI [main, develop] 6
eIPC [main, develop] 6
eDB [main, develop] 4
eBrowser [main, develop] 4
eOffice [main, develop] 0

eApps is the one that got it right — [main, master, develop].

I will open the same change against the others once this shape is agreed, rather
than raising six PRs on an approach nobody has looked at yet.

This is the third hardcoded-name failure found this week, after ebuild#81
("branch": "main" for repositories whose default is master) and EoSim#16
(a lowercase repo list finding 2 of 19 on a case-sensitive filesystem).

ci.yml watched `main` and `develop`. Neither exists:

    $ gh api repos/embeddedos-org/eAI -q .default_branch
    master
    $ gh api repos/embeddedos-org/eAI/branches/main
    404 Branch not found

So every push to master and every pull request against it — they all target
master — fell outside the trigger. The last run was 2026-05-31, and the
pattern shows what happened:

    2026-05-31  branch=v1.9.0  failure
    2026-05-28  branch=v1.5.0  failure
    2026-05-27  branch=main    failure
    2026-05-27  branch=main    success

main existed until late May, the workflow ran on it, the branch was renamed
to master, and the workflow was left pointing at a branch that had gone. The
runs after that are tag pushes, which still match tags: ["v*"], and all
three failed.

ci.yml is the only workflow here that compiles the C, runs pytest and
cross-builds for ARM:

    jobs:
      test-c:
      test-python:
      build-arm:
      release:

Five other workflows do watch master and have kept running — codeql,
book-build, video-build, deploy-pages, scorecard — so the repository has not
been silent. But none of them is a test gate. codeql builds, as input to
static analysis, not to check the build.

master is added rather than substituted, so a later rename in either
direction does not break this again.

Expect the first run to be red. The last three were, and three months of
changes have landed unverified since; finding that out is the point.

Refs #38

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@srpatcha

Copy link
Copy Markdown
Member Author

CI is running again, and it found something within a minute. Results on this PR:

C/C++ Tests                pass   32s
Analyze (python)           pass   33s
assign                     pass    4s
Python Tests               FAIL   12s
Cross-compile ARM          skipping
Create GitHub Release      skipping

So the C side is healthy and the Python suite cannot collect at all:

tests/unit/test_unit_core.py:10: import numpy as np
E   ModuleNotFoundError: No module named 'numpy'
Interrupted: 1 error during collection

numpy is declared nowhere — no requirements.txt, nothing in packaging — and
ci.yml:45 hides it:

pip install -r requirements.txt 2>/dev/null || true

The file does not exist, and || true turns that into a silent success. Filed
as #40, kept out of this PR so the trigger change stays reviewable on its own.

This red is the point of the PR, not a regression from it. The failure has
been there since May; the only new thing is that someone can see it. I said to
expect this in the description, and it is worth being concrete now that it has
happened: merging this makes the repository's CI honest, and the immediate
consequence is a red badge until #40 is fixed.

If you would rather land a green master first, #40 is a two-line change and
could go in ahead of this. I would take them the other way round — a red gate
that reflects reality is more useful than a green one that has not run since May.

The same change is now open on the five sibling repositories with the identical
trigger bug: eNI#30, eIPC#31, eDB#70, eBrowser#20, eOffice#40. Each is likely to
surface its own first failure the same way.

@srpatcha

Copy link
Copy Markdown
Member Author

Correction to my comment above, prompted by #41.

I reported C/C++ Tests pass as evidence the C side was healthy. That pass
was meaningless.
The job was building zero tests.

CMakeLists.txt:16 defines EAI_BUILD_TESTS; the workflow passed
-DBUILD_TESTS=ON, which is not an option this project has. Measured:

-DBUILD_TESTS=ON       ->  No tests were found!!!
-DEAI_BUILD_TESTS=ON   ->  100% tests passed, 0 tests failed out of 24

and the old invocation ended in || true, so ctest finding nothing exited 0 and
the job went green:

ctest, zero tests, `|| true`   ->  exit=0

So this repository had a green C/C++ badge over a suite that was never compiled —
twenty-four tests that have never run in CI.

Restoring the trigger here made the job run. #41 makes it mean something, by
using the right option name, dropping || true, and adding --no-tests=error
so an empty run is a failure rather than a pass. I have approved it.

The two changes are complementary and neither is sufficient alone: without this
PR the workflow does not fire at all; without #41 it fires and reports success
regardless. Worth merging both, and #41 second so the first honest run happens
with the guard already in place.

The rest of my comment stands — Python Tests fails on an undeclared numpy
(#40), and that failure is real.

srpatcha added a commit to embeddedos-org/eDB that referenced this pull request Sep 1, 2026
ci.yml watched `main` and `develop`. Neither exists — this repository's
default branch is `master`, so every push to it and every pull request
against it fell outside the trigger.

Last run of ci.yml: 2026-05-31. A repository-wide rename from main to master in
late May left the workflow pointing at a branch that had gone, and nothing
has built or tested a change here since.

master is added rather than substituted, on both push and pull_request, so a
rename in either direction does not break this again.

Expect the first run to be red. Three months of changes have landed
unverified; finding that out is the point.

Same fix as embeddedos-org/eAI#39, where it is verified to work: the PR went
from a single skipped `assign` job to `C/C++ Tests` and `Python Tests`
actually running.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
srpatcha added a commit to embeddedos-org/eOffice that referenced this pull request Sep 1, 2026
ci.yml watched `main` and `develop`. Neither exists — this repository's
default branch is `master`, so every push to it and every pull request
against it fell outside the trigger.

Last run of ci.yml: 2026-08-28. A repository-wide rename from main to master in
late May left the workflow pointing at a branch that had gone, and nothing
has built or tested a change here since.

master is added rather than substituted, on both push and pull_request, so a
rename in either direction does not break this again.

Expect the first run to be red. Three months of changes have landed
unverified; finding that out is the point.

Same fix as embeddedos-org/eAI#39, where it is verified to work: the PR went
from a single skipped `assign` job to `C/C++ Tests` and `Python Tests`
actually running.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

@srpatcha srpatcha left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Review — eAI#39 "ci: run the build-and-test workflow on master"

head: 9dbf01a author: srpatcha ci: fail (Python Tests)

Verdict: Two-line trigger repair, correct and evidenced; it conforms. One thing has
changed under it since the discussion above, and one fake-green survives the merge.

Findings

# Severity File:line Finding Recommended fix
1 Medium .github/workflows/ci.yml:62-67 (master) After this merges, the workflow finally fires — and build-arm is still fully suppressed: cmake ... 2>/dev/null || true twice, then an unconditional echo "ARM build complete" > dist/build.txt. It also names cmake/arm-cortex-m4.cmake, which does not exist (the repo has cmake/toolchain-arm-cortex-m4.cmake and cmake/toolchains/toolchain-arm-cortex-m4.cmake). The job cannot fail and uploads a marker asserting a build that never happened. Land #41's ARM hunk with this, or immediately after. Not a change to make inside this PR.
2 Low PR body, "The change" YAML validated. is the one claim here with no command output behind it. Everything else in the body is backed; this line is not. Quote the validator invocation, or drop the claim.

Correction to the record, not a finding

The second comment on this PR reports that -DBUILD_TESTS=ON and the trailing || true
still make the C/C++ badge meaningless. That half is already on master — commit
abd678b (merge of #33, chore/audit-and-ci). Current origin/master:.github/workflows/ci.yml
reads:

            -DEAI_BUILD_TESTS=ON
          cmake --build build --parallel $(nproc)
          # No `|| true`: a failing test must fail the job. --no-tests=error
          # catches an empty test set, which ctest otherwise reports as success.
          cd build && ctest --output-on-failure --no-tests=error

The C/C++ Tests pass 32s in this PR's checks.txt came from the old recipe, because
the PR head branches from 21d8b0a. So the first honest master run after this merges
will use the already-fixed C recipe — the guard is in place, and #41's remaining value is
the ARM half (finding 1), not the C half.

master's on: block is still branches: [main, develop] / pull_request: [main], so
this PR is still needed and still correct, and gh reports it MERGEABLE.

CI state

Python Tests fail (12s) — ModuleNotFoundError: numpy, tracked as #40 and already
argued out above; not repeated here. Cross-compile ARM Cortex-M4 and Create GitHub Release skip behind it. Analyze (python), C/C++ Tests, assign pass.
mergeStateStatus: BLOCKED follows from the red required check.

Architecture conformance

Conforms. Master design §21 places eAI in Tier 3 (Advanced); a CI trigger list is
Infrastructure inside the owning repo, which is where §21.1 wants it. No dependency
edge is added, so §5.1 is untouched — nothing here makes a lower tier reference a higher
one.

Proposed changes

None to this diff. Merge order that keeps each step honest:

  1. This PR — the workflow starts firing on master.
  2. #41 rebased to its ARM hunk only — build-arm stops reporting success for a build it
    did not do.
  3. #40numpy declared, Python Tests goes green for a real reason.

Not checked

  • I did not run the workflow. CI state is read from the bundle's checks.txt only.
  • prepare-run.sh produced an empty diff.patch for this bundle; gh pr diff in the
    bundler swallows its own errors with 2>/dev/null. The diff reviewed here was
    re-fetched by hand. Worth fixing in the tooling — an empty diff currently looks like
    "no changes" rather than "fetch failed".
  • I did not verify the five sibling repos in the body's table.

Automated architecture review of 9dbf01a08c4a — scheduled, model claude-opus-5, checked against the EmbeddedOS Master Design v2.0. Advisory only: this reviewer never approves, requests changes, or merges. Reply here to discuss or push back — a wrong finding is a bug worth reporting.

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.

CI has not run since May — ci.yml watches main and develop, neither of which exists

1 participant