Skip to content

A small update on running tests docs #1219

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.

Sign up for GitHub

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

Merged
Kipok merged 3 commits into from
Feb 6, 2026
Merged

A small update on running tests docs #1219

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6f94020
A small update on running tests docs
Kipok Feb 6, 2026
c8883e1
Exclude agents md from docs for now
Kipok Feb 6, 2026
0485b12
Enable incremental review
Kipok Feb 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .coderabbit.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ reviews:
# Default: {}
# Automatic Incremental Review - Automatic incremental code review on each push
# Default: true
auto_incremental_review: false # current: true
auto_incremental_review: true # current: true
# Review draft PRs/MRs.
# Default: false
drafts: false
Expand Down
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ nemo_skills/dataset/aalcr/lcr/
.idea/
.idea/*
CLAUDE.md
AGENTS.md
.codex

.idea
6 changes: 5 additions & 1 deletion CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,11 @@ All subsequent commits will be checked according to configuration in [`.pre-comm

## Running Tests

TBD
Check existing github actions CI for cpu/gpu tests. Slurm tests documentation is [here](/tests/slurm-tests).
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if /tests/slurm-tests exists as a directory or file
fd -t d 'slurm-tests' tests/ 2>/dev/null || echo "No directory named slurm-tests found"
fd -t f 'slurm-tests*' tests/ 2>/dev/null || echo "No files matching slurm-tests* found"
# Also check for any README or index within tests directory
find tests/ -maxdepth 2 -name "*slurm*" 2>/dev/null || echo "No slurm-related files found in tests/"
# List what's actually in tests/ directory to understand structure
ls -la tests/ 2>/dev/null | head -20

Repository: NVIDIA-NeMo/Skills

Length of output: 1373

Capitalize "GitHub" and use descriptive link text.

📝 Proposed fix 
-Check existing github actions CI for cpu/gpu tests. Slurm tests documentation is [here](/tests/slurm-tests).
+Check existing GitHub Actions CI for cpu/gpu tests. See the [Slurm tests documentation](/tests/slurm-tests).
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Check existing github actions CI for cpu/gpu tests. Slurm tests documentation is [here](/tests/slurm-tests).
Check existing GitHub Actions CI for cpu/gpu tests. See the [Slurm tests documentation](/tests/slurm-tests).
🧰 Tools
🪛 LanguageTool

[uncategorized] ~166-~166: The official name of this software platform is spelled with a capital “H”.
Context: ... ``` ## Running Tests Check existing github actions CI for cpu/gpu tests. Slurm tes...

(GITHUB)

🪛 markdownlint-cli2 (0.20.0)

[warning] 166-166: Link text should be descriptive

(MD059, descriptive-link-text)

🤖 Prompt for AI Agents 
In `@CONTRIBUTING.md` at line 166, Update the sentence to use the proper noun
"GitHub Actions" instead of "github actions" and change the link text to be
descriptive by replacing "Slurm tests documentation is
[here](/tests/slurm-tests)" with "Slurm tests documentation" as the link text
(e.g., "Check existing GitHub Actions CI for cpu/gpu tests. Slurm tests
documentation: [Slurm tests documentation](/tests/slurm-tests)").

When running cpu tests, it's important to always add `-s` flag to pytest as otherwise all tests using nemo-run
will fail.
Comment on lines +166 to +168
Copy link
Contributor

@greptile-apps greptile-apps bot Feb 6, 2026

Choose a reason for hiding this comment

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

Incorrect pytest flag

This says to add pytest’s -s flag, but -s is a git commit -s/--signoff convention and in pytest it disables output capture (not generally required for tests to pass). If the intent is to avoid failures due to missing stdout capture, that’s unlikely; if the intent is to stop at first failure or increase verbosity, the flags should be -x / -vv. As written, contributors may run the wrong command and still get failing tests.

Consider replacing this with the exact pytest ... invocation you expect contributors to run (including any required env vars), or remove the -s guidance unless it’s truly required for correctness.


More details TBD

**TIP**: Our CI depends on some secret variables only accessible to developers of the repository.
To run the full suite of tests, please create pull requests from a branch instead of a fork whenever
Expand Down