Run generator_test.py in CI checks

[rolandreichweinbmw]

We currently have 2 facilities for testing generators: Running generator.bat directly and checking the result, and generator_test.py. Only the first one was run in the CI checks, silently missing if the second one failed. Now adding the second one also to the workflow.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: cc4d2487-446a-412d-895e-0fa7a11f2243

📥 Commits

Reviewing files that changed from the base of the PR and between ac23e7d and 0e2c633.

📒 Files selected for processing (1)

• .github/workflows/generator.yml

✅ Files skipped from review due to trivial changes (1)

• .github/workflows/generator.yml

---

Walkthrough

The CI workflow was updated to replace the previous “Generate” step and the “Check Generated Headers For Changes” git diff --exit-code check with an Install dependencies step that installs python3-cogapp and a Run generator_test.py step that changes into the scripts directory and runs python3 generator_test.py. No other workflow logic or conditionals were modified.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately reflects the main change: adding generator_test.py execution to CI checks instead of just the generator.bat validation.
Description check	✅ Passed	The description clearly explains the rationale and changes: two testing facilities existed but only one was run in CI, and now both are included to catch failures.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

.github/workflows/generator.yml (1)

17-25: ⚠️ Potential issue | 🔴 Critical

Missing cogapp dependency causes step failure.

The generator_test.py script invokes python3 -m cogapp internally (see scripts/generator_test.py lines 21-31), but python3-cogapp is only installed in the subsequent "Generate" step at line 24. This step will fail with a ModuleNotFoundError for cogapp.

Move the cogapp installation before running the test script:

Proposed fix

     - uses: actions/checkout@v4
 
+    - name: Install cogapp
+      run: |
+        sudo apt-get update
+        sudo apt-get install -y python3-cogapp
+
     - name: Run generator_test.py
       run: |
         cd scripts && python3 generator_test.py
 
     - name: Generate
       run: |
-        sudo apt-get update
-        sudo apt-get install -y python3-cogapp
         cd include/etl/generators && bash generate.bat
 
     - name: Check Generated Headers For Changes

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/generator.yml around lines 17 - 25, The workflow installs
python3-cogapp too late, causing generator_test.py (which calls python3 -m
cogapp) to fail with ModuleNotFoundError; move the cogapp installation so that
the "Generate" dependency (installing python3-cogapp) runs before the "Run
generator_test.py" step (or combine/install python3-cogapp at the top of the
job) so generator_test.py can import cogapp successfully—update the steps that
reference the "Run generator_test.py" action and the "Generate" action
accordingly.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In @.github/workflows/generator.yml:
- Around line 17-25: The workflow installs python3-cogapp too late, causing
generator_test.py (which calls python3 -m cogapp) to fail with
ModuleNotFoundError; move the cogapp installation so that the "Generate"
dependency (installing python3-cogapp) runs before the "Run generator_test.py"
step (or combine/install python3-cogapp at the top of the job) so
generator_test.py can import cogapp successfully—update the steps that reference
the "Run generator_test.py" action and the "Generate" action accordingly.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: fc4d9a5c-9207-429f-9e5e-2ead7ccedf51

📥 Commits

Reviewing files that changed from the base of the PR and between 912e22f and acd3be8.

📒 Files selected for processing (1)

• .github/workflows/generator.yml

[jwellbelove]

Actually, I'm not sure that generate.bat should actually be run at all at CI.
I think that generator_test is only one that should be run, as what we are really trying to do is ensure the generators would exactly re-generate the existing files.

[rolandreichweinbmw]

Agreed. Removed generator.bat, kept generator_test.py in CI check.