Skip to content

feat(scripts): add npm run scripts for Python linting and testing#933

Closed
JasonOA888 wants to merge 1 commit intomicrosoft:mainfrom
JasonOA888:feat/issue-886-npm-python-scripts
Closed

feat(scripts): add npm run scripts for Python linting and testing#933
JasonOA888 wants to merge 1 commit intomicrosoft:mainfrom
JasonOA888:feat/issue-886-npm-python-scripts

Conversation

@JasonOA888
Copy link
Contributor

@JasonOA888 JasonOA888 commented Mar 8, 2026

Description

Add npm run scripts for Python linting and testing:

  • lint:py — ruff check
  • lint:py:fix — ruff check --fix
  • test:py — pytest

Follows existing npm script patterns.

Related Issue(s)

Fixes #886

Type of Change

Select all that apply:

Code & Documentation:

  • Bug fix (non-breaking change fixing an issue)
  • New feature (non-breaking change adding functionality)
  • Breaking change (fix or feature causing existing functionality to change)
  • Documentation update

Infrastructure & Configuration:

  • GitHub Actions workflow
  • Linting configuration (markdown, PowerShell, etc.)
  • Security configuration
  • DevContainer configuration
  • Dependency update

AI Artifacts:

  • Reviewed contribution with prompt-builder agent and addressed all feedback
  • Copilot instructions (.github/instructions/*.instructions.md)
  • Copilot prompt (.github/prompts/*.prompt.md)
  • Copilot agent (.github/agents/*.agent.md)
  • Copilot skill (.github/skills/*/SKILL.md)

Note for AI Artifact Contributors:

  • Agents: Research, indexing/referencing other project (using standard VS Code GitHub Copilot/MCP tools), planning, and general implementation agents likely already exist. Review .github/agents/ before creating new ones.
  • Skills: Must include both bash and PowerShell scripts. See Skills.
  • Model Versions: Only contributions targeting the latest Anthropic and OpenAI models will be accepted. Older model versions (e.g., GPT-3.5, Claude 3) will be rejected.
  • See Agents Not Accepted and Model Version Requirements.

Other:

  • Script/automation (.ps1, .sh, .py)
  • Other (please describe):

Testing

Checklist

Required Checks

  • Documentation is updated (if applicable)
  • Files follow existing naming conventions
  • Changes are backwards compatible (if applicable)
  • Tests added for new functionality (if applicable)

Required Automated Checks

The following validation commands must pass before merging:

  • Markdown linting: npm run lint:md
  • Spell checking: npm run spell-check
  • Frontmatter validation: npm run lint:frontmatter
  • Skill structure validation: npm run validate:skills
  • Link validation: npm run lint:md-links
  • PowerShell analysis: npm run lint:ps

- Add lint:py for ruff check
- Add lint:py:fix for ruff check --fix
- Add test:py for pytest

Follows existing npm run script patterns for consistency.

Fixes microsoft#886
@JasonOA888 JasonOA888 requested a review from a team as a code owner March 8, 2026 06:11
@WilliamBerryiii
Copy link
Member

Hi @JasonOA888 — thank you for taking the time to contribute to this project! We really appreciate the effort you've put into this PR.

Before we can review and merge your changes, we need you to sign the Microsoft Contributor License Agreement (CLA). The CLA bot should have posted instructions on this PR when it was opened. You can sign by posting the following comment:

@microsoft-github-policy-service agree

If you're contributing on behalf of a company, use:

@microsoft-github-policy-service agree company="{your company}"

We'd love to include your work, but unfortunately we won't be able to accept the contribution without a signed CLA. If we don't hear back, we'll eventually need to close this PR and potentially redo the work under a signed agreement.

Thanks again, and please let us know if you have any questions!

@WilliamBerryiii
Copy link
Member

PR Compliance Update — The PR body was reformatted to use the project's PR template with all required sections (Related Issue(s), Type of Change, Checklist, Security Considerations, etc.). The original description and Fixes #886 reference were preserved.

No content was changed — only formatting alignment. Thank you for the contribution! 🙏

"lint:frontmatter": "pwsh -NoProfile -Command \"& './scripts/linting/Validate-MarkdownFrontmatter.ps1' -WarningsAsErrors -EnableSchemaValidation\"",
"lint:collections-metadata": "pwsh -File scripts/collections/Validate-Collections.ps1",
"lint:marketplace": "pwsh -File scripts/plugins/Validate-Marketplace.ps1",
"lint:py": "ruff check .",
Copy link
Contributor

Choose a reason for hiding this comment

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

@JasonOA888 thanks for taking the time for creating a contribution.
We noticed there are a few small things missing to align the change with the issue description: primarily each of these new additions should ensure dynamic discovery of each Skill using Python through their pyproject.yaml files, using find. Could you update your PR to match the issue requirements?

Thanks for the alphabetical ordering, this is helpful too!

@JasonOA888
Copy link
Contributor Author

@microsoft-github-policy-service agree

@JasonOA888
Copy link
Contributor Author

Thanks for the review @katriendg and @WilliamBerryiii!

The CLA has been signed.

Regarding the dynamic discovery of Python Skills using find - could you clarify what you'd like to see? The current PR adds simple npm scripts for Python linting/testing:

  • lint:pyruff check .
  • test:pypytest

Are you asking for these commands to dynamically discover Python files rather than running on .? Or is this about discovering Skills that use Python from their pyproject.yaml files?

Happy to update the PR once I understand the requirements better.

@WilliamBerryiii
Copy link
Member

WilliamBerryiii commented Mar 9, 2026

Thanks for the review @katriendg and @WilliamBerryiii!

The CLA has been signed.

Regarding the dynamic discovery of Python Skills using find - could you clarify what you'd like to see? The current PR adds simple npm scripts for Python linting/testing:

  • lint:pyruff check .
  • test:pypytest

Are you asking for these commands to dynamically discover Python files rather than running on .? Or is this about discovering Skills that use Python from their pyproject.yaml files?

Happy to update the PR once I understand the requirements better.

@JasonOA888 I'll get you an example in a min ... basically getting ruff mapped into all the various skills folders that have python in them. Obviously, right now we only have 1 skill still in PR form with python code in it, but we are anticipating needed to support a TON of skills where python is the backing code for the skill ... so we want to make sure the CLI commands for devs, go and find all the skills that py project toml files and lints them all. (Hope that clears things up)

JasonOA888 added a commit to JasonOA888/hve-core that referenced this pull request Mar 9, 2026
Implements dynamic discovery of Python skills by finding all
pyproject.toml files and running ruff/pytest on each.

## Changes

- Add Invoke-PythonLint.ps1 - discovers Python skills and runs ruff
- Add Invoke-PythonTests.ps1 - discovers Python skills and runs pytest
- Update package.json scripts:
  - lint:py -> uses Invoke-PythonLint.ps1
  - test:py -> uses Invoke-PythonTests.ps1

## How it works

1. Scans repository for pyproject.toml files (excludes node_modules)
2. For each Python skill directory:
   - lint:py runs ruff check
   - test:py runs pytest (if tests/ exists)
3. Reports summary of passed/failed skills

## Example output

```
Found 3 Python skill(s):
  - ./skills/my-python-skill
  - ./skills/another-skill

Running ruff in ./skills/my-python-skill...
✓ No linting issues

Running ruff in ./skills/another-skill...
❌ Linting issues found

❌ Linting completed with errors
```

Addresses feedback from microsoft#933
@JasonOA888
Copy link
Contributor Author

Thanks for the clarification @WilliamBerryiii!

I've created a new PR that implements dynamic discovery of Python skills:

PR: #935

Implementation

The new approach:

  1. Scans for pyproject.toml files (excludes node_modules)
  2. Runs ruff/pytest in each discovered directory
  3. Reports summary of passed/failed skills

Scripts Added

scripts/linting/
├── Invoke-PythonLint.ps1    # Dynamic ruff linting
└── Invoke-PythonTests.ps1   # Dynamic pytest testing

Example Output

Found 3 Python skill(s):
  - ./skills/my-python-skill
  - ./skills/another-skill
  
Running ruff in ./skills/my-python-skill...
✓ No linting issues

Running ruff in ./skills/another-skill...
❌ Linting issues found

❌ Linting completed with errors

package.json Updates

{
  "lint:py": "pwsh -NoProfile -File ./scripts/linting/Invoke-PythonLint.ps1",
  "test:py": "pwsh -NoProfile -File ./scripts/linting/Invoke-PythonTests.ps1"
}

This design will automatically discover and lint/test ALL Python skills as they're added, without needing to update the npm scripts.

Closing this PR in favor of #935.

@WilliamBerryiii
Copy link
Member

Ref #957 as the replacement! Closing this one

WilliamBerryiii added a commit that referenced this pull request Mar 14, 2026
Implements dynamic discovery of Python skills by finding all
`pyproject.toml` files and running ruff/pytest on each.

## Motivation

As the project grows to support more Python skills, we need a way to
automatically discover and lint/test all of them without manually
updating npm scripts.

## Changes

### New Scripts

```
scripts/linting/
├── Invoke-PythonLint.ps1    # Discovers Python skills, runs ruff
└── Invoke-PythonTests.ps1   # Discovers Python skills, runs pytest
```

### How It Works

1. **Scans repository** for `pyproject.toml` files (excludes
`node_modules`)
2. **For each Python skill**:
   - `lint:py` runs `ruff check`
   - `test:py` runs `pytest` (if `tests/` directory exists)
3. **Reports summary** of passed/failed skills

### Example Output

```
Found 3 Python skill(s):
  - ./skills/my-python-skill
  - ./skills/another-skill
  
Running ruff in ./skills/my-python-skill...
✓ No linting issues

Running ruff in ./skills/another-skill...
❌ Linting issues found

❌ Linting completed with errors
```

### package.json

```json
{
  "lint:py": "pwsh -NoProfile -File ./scripts/linting/Invoke-PythonLint.ps1",
  "test:py": "pwsh -NoProfile -File ./scripts/linting/Invoke-PythonTests.ps1"
}
```

## Benefits

- **Automatic discovery**: New Python skills are automatically included
- **Cross-platform**: PowerShell scripts work on Windows/Linux/macOS
- **Clear output**: Shows which skills passed/failed
- **Fail-fast**: Returns non-zero exit code on any failure

## Testing

- Tested with multiple `pyproject.toml` files
- Handles missing ruff/pytest gracefully
- Excludes node_modules correctly

Addresses feedback from #933

Closes #933

---------

Co-authored-by: Bill Berry <WilliamBerryiii@users.noreply.github.com>
Co-authored-by: Bill Berry <wberry@microsoft.com>
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.

feat(scripts): Add npm run scripts for Python linting and testing

3 participants