Skip to content

Commit

Permalink
Ruff changes, expand pre-commit hooks to checkPot and unit tests (#16767
Browse files Browse the repository at this point in the history
)

Fix up for #16751

Summary of the issue:
Ruff recently updated, so we might as well use the latest version.
a warning is omitted for the section we added logger-objects to in ruff's config.
it was unclear in docs how to avoid pre-commit hooks from triggering.
more pre-commit hooks could be added
ruff whitespace formating is done through ruff format currently we are only linting with ruff check. We should also run a ruff format on the whole NVDA.
ruff was missing scons files, which should also be linted in a mass lint.
Summary of other changes:
Added pre commit hooks for unit tests and translation comment checks.
Added generic pre-commit hooks for python static syntax checking, basic whitespace rules, and file write safety checks
change the output of rununittests.bat to have simpler buffer output, only show failures. Ensure the verbose log is logged in AppVeyor.
add ruff format to lint checks
add scons files to lint checks
update coderabbit prompt for change log entry items
  • Loading branch information
seanbudd authored Jul 3, 2024
1 parent 97a994b commit b47fdc5
Show file tree
Hide file tree
Showing 9 changed files with 76 additions and 32 deletions.
11 changes: 11 additions & 0 deletions .coderabbit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,17 @@ reviews:
Also consider readability and clarity of contents.
Ensure that changes follow the style of one sentence per line, suggest changes if this is not met.
"
- path: "user_docs/en/changes.md"
instructions: "
Ensure each change log entry references an issue or pull request number.
Change log entries can also include a reference to a GitHub author.
Examples of valid change log entries:
* Item with sub-items (#123, @username):
* sub-item
* bar (#342)
* Item with.
Multiple lines. (#143)
"
tools:
github-checks:
enabled: true
Expand Down
31 changes: 27 additions & 4 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,16 +1,39 @@
repos:

- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.6.0
hooks:
# Checks python syntax
- id: check-ast
- id: check-case-conflict
- id: check-merge-conflict
- id: end-of-file-fixer
- id: trailing-whitespace

- repo: https://github.com/astral-sh/ruff-pre-commit
# Ruff version.
rev: v0.4.10
# Matches Ruff version in requirements.
rev: v0.5.0
hooks:
# Run the linter.
- id: ruff
name: lint with ruff
args: [ --fix ]
# Run the formatter.
- id: ruff-format
name: format with ruff

- repo: local
hooks:
- id: checkPot
name: translation string check
entry: cmd.exe /c "scons checkPot -j 4"
language: system
pass_filenames: false
types: [python, pofile]
- id: unitTest
name: unit tests
entry: ./rununittests.bat
language: script
pass_filenames: false
types: [python, c, c++, batch]
- id: licenseCheck
name: Check license compatibility of pip dependencies
files: requirements.txt
Expand Down
8 changes: 4 additions & 4 deletions appveyor/scripts/tests/lintCheck.ps1
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
$lintOutput = (Resolve-Path .\testOutput\lint\)
$lintOutput = "$lintOutput\PR-lint.xml"
.\runlint.bat "$lintOutput"
if ($LastExitCode -ne 0) {
Set-AppveyorBuildVariable "testFailExitCode" $LastExitCode
Add-AppveyorMessage "FAIL: Lint check. See test results for more information."
Add-AppveyorMessage "FAIL: Lint check. See test results and lint artifacts for more information."
} else {
Add-AppveyorMessage "PASS: Lint check."
}
Push-AppveyorArtifact $lintOutput
Push-AppveyorArtifact "$lintOutput/PR-lint.xml"
Push-AppveyorArtifact "$lintOutput/lint-diff.diff"
$wc = New-Object 'System.Net.WebClient'
$wc.UploadFile("https://ci.appveyor.com/api/testresults/junit/$($env:APPVEYOR_JOB_ID)", $lintOutput)
$wc.UploadFile("https://ci.appveyor.com/api/testresults/junit/$($env:APPVEYOR_JOB_ID)", "$lintOutput/PR-lint.xml")
2 changes: 1 addition & 1 deletion appveyor/scripts/tests/unitTests.ps1
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
$outDir = (Resolve-Path .\testOutput\unit\)
$unitTestsXml = "$outDir\unitTests.xml"
.\rununittests.bat --output-file "$unitTestsXml"
.\rununittests.bat --output-file "$unitTestsXml" -v
if($LastExitCode -ne 0) {
Set-AppveyorBuildVariable "testFailExitCode" $LastExitCode
Add-AppveyorMessage "FAIL: Unit tests. See test results for more information."
Expand Down
15 changes: 0 additions & 15 deletions projectDocs/dev/lint.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,3 @@ Our linting process involves running [Ruff](https://docs.astral.sh/ruff) to pick
## Lint integration

For faster lint results, or greater integration with your tools you may want to set up Ruff with your IDE.

## Pre-commit hooks

[Pre-commit hooks](https://pre-commit.com/) can be used to automatically run linting on files staged for commit.
This will automatically apply lint fixes where possible, otherwise cancelling the commit on lint issues.

From a shell, set up pre-commit scripts for your NVDA python environment:

1. `venvUtils\ensureAndActivate.bat`
1. `pre-commit install`

Alternatively, set up pre-commit scripts globally:

1. `pip install pre-commit`
1. `pre-commit install --allow-missing-config`
20 changes: 20 additions & 0 deletions projectDocs/testing/automated.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,28 @@
## Running Automated Tests

If you make a change to the NVDA code, you should run NVDA's automated tests.
These tests help to ensure that code changes do not unintentionally break functionality that was previously working.

### Pre-commit hooks

[Pre-commit hooks](https://pre-commit.com/) can be used to automatically run linting, translatable string checks and unit tests on files staged for commit.
This will automatically apply lint fixes where possible, and will cancel the commit on lint issues and other test failures.

From a shell, set up pre-commit scripts for your NVDA python environment:

1. `venvUtils\ensureAndActivate.bat`
1. `pre-commit install`

Alternatively, set up pre-commit scripts globally:

1. `pip install pre-commit`
1. `pre-commit install --allow-missing-config`

To skip pre-commit hooks from triggering, use the `--no-verify` CLI option.
Example: `git commit -m "message" --no-verify`.

### Translatable string checks

To run the translatable string checks (which check that all translatable strings have translator comments), run:

```cmd
Expand Down
6 changes: 3 additions & 3 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@ builtins = [
"npgettext",
]

logger-objects = ["logHandler.log"]

include = [
"*.py",
"*.pyw",
"sconstruct",
"*sconscript",
]

exclude = [
Expand Down Expand Up @@ -51,6 +51,7 @@ ignore = [
# indentation contains tabs
"W191",
]
logger-objects = ["logHandler.log"]

[tool.licensecheck]
using = "requirements:requirements.txt"
Expand Down Expand Up @@ -79,4 +80,3 @@ ignore_packages = [
"wxPython", # wxWindows Library License
"pillow", # PIL Software License
]

13 changes: 9 additions & 4 deletions runlint.bat
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
@echo off
rem runlint [<output file>]
rem Lints the entire repository
rem runlint [<output dir>]
rem Lints and formats all python files
set hereOrig=%~dp0
set here=%hereOrig%
if #%hereOrig:~-1%# == #\# set here=%hereOrig:~0,-1%
set scriptsDir=%here%\venvUtils

if "%1" NEQ "" set ruffArgs=--output-file=%1 --output-format=junit
call "%scriptsDir%\venvCmd.bat" ruff check --fix %ruffArgs%
set ruffCheckArgs=
set ruffFormatArgs=
if "%1" NEQ "" set ruffCheckArgs=--output-file=%1/PR-lint.xml --output-format=junit
if "%1" NEQ "" set ruffFormatArgs=--diff > %1/lint-diff.diff
call "%scriptsDir%\venvCmd.bat" ruff check --fix %ruffCheckArgs%
if ERRORLEVEL 1 exit /b %ERRORLEVEL%
call "%scriptsDir%\venvCmd.bat" ruff format %ruffFormatArgs%
if ERRORLEVEL 1 exit /b %ERRORLEVEL%
2 changes: 1 addition & 1 deletion rununittests.bat
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ set unitTestsPath=%here%\tests\unit
set testOutput=%here%\testOutput\unit
md %testOutput%

call "%scriptsDir%\venvCmd.bat" py -m xmlrunner discover -v -s "%unitTestsPath%" -t "%here%" --output-file "%testOutput%\unitTests.xml" %*
call "%scriptsDir%\venvCmd.bat" py -m xmlrunner discover -b -s "%unitTestsPath%" -t "%here%" --output-file "%testOutput%\unitTests.xml" %*

0 comments on commit b47fdc5

Please sign in to comment.