-
Notifications
You must be signed in to change notification settings - Fork 48
feat: run cogames smoke test from source code in child repo #4523
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: yatharth/cogames-release-order
Are you sure you want to change the base?
feat: run cogames smoke test from source code in child repo #4523
Conversation
There was a problem hiding this comment.
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.
ℹ️ 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".
| - uses: actions/checkout@v4 | ||
| with: | ||
| repository: Metta-AI/cogames | ||
| # For tag pushes: use the tag name (e.g., cogames-v1.2.3) | ||
| # For manual dispatch: use main branch (since arbitrary branches aren't in child repo) | ||
| ref: ${{ startsWith(github.ref, 'refs/tags/') && github.ref_name || 'main' }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Align mettagrid wait with the repo under test
The test job now checks out Metta-AI/cogames (lines 63–68), but the wait-for-mettagrid job still extracts the required mettagrid version from packages/cogames/pyproject.toml in this repo (lines 102–107). If the child repo’s dependency pin diverges from the monorepo’s copy, the workflow may wait for the wrong version (or not wait at all), causing intermittent install failures or false confidence. This mismatch is introduced by the new checkout of the child repo without updating the wait step’s source of truth.
Useful? React with 👍 / 👎.
| - name: Run tests | ||
| run: | | ||
| # Run from clean directory - no metta/ or other repo packages accessible | ||
| pytest tests/ -v |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Smoke test no longer validates the installed package
Running pytest tests/ -v from the repository root after pip install . means Python will import from the working tree first (cwd is at the front of sys.path). This can mask packaging regressions (e.g., missing package data, incorrect package includes, or build-time generated files) because the tests may exercise the source tree instead of the installed distribution. The previous isolation step avoided this; the new flow no longer validates that the install artifact is actually runnable.
Useful? React with 👍 / 👎.
cdbd915 to
f859d89
Compare

Prompted by nishad's comment on #4423.
(example of successful run)