Skip to content

Commit dfe787d

Browse files
authored
Merge 292d070 into 75c8d00
2 parents 75c8d00 + 292d070 commit dfe787d

12 files changed

Lines changed: 341 additions & 9 deletions

File tree

appveyor.yml

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ environment:
2323
init:
2424
- ps: |
2525
# iex ((new-object net.webclient).DownloadString('https://raw.githubusercontent.com/appveyor/ci/master/scripts/enable-rdp.ps1'))
26+
$pythonVersion = (py --version)
27+
echo $pythonVersion
2628
if ($env:APPVEYOR_REPO_TAG_NAME -and $env:APPVEYOR_REPO_TAG_NAME.StartsWith("release-")) {
2729
# Strip "release-" prefix.
2830
$version = $env:APPVEYOR_REPO_TAG_NAME.Substring(8)
@@ -121,10 +123,11 @@ build_script:
121123

122124
before_test:
123125
# install required packages
124-
- py -m pip install -r tests/system/requirements.txt
126+
- py -m pip install -r tests/system/requirements.txt -r tests/lint/lintInstall/requirements.txt
125127
- mkdir testOutput
126128
- mkdir testOutput\unit
127129
- mkdir testOutput\system
130+
- mkdir testOutput\lint
128131
- ps: |
129132
$errorCode=0
130133
$nvdaLauncherFile=".\output\nvda"
@@ -157,6 +160,28 @@ test_script:
157160
$wc = New-Object 'System.Net.WebClient'
158161
$wc.UploadFile("https://ci.appveyor.com/api/testresults/junit/$($env:APPVEYOR_JOB_ID)", $unitTestsXml)
159162
if($errorCode -ne 0) { $host.SetShouldExit($errorCode) }
163+
- ps: |
164+
if($env:APPVEYOR_PULL_REQUEST_NUMBER) {
165+
$lintOutput = (Resolve-Path .\testOutput\lint\)
166+
$lintSource = (Resolve-Path .\tests\lint\)
167+
git fetch -q origin $env:APPVEYOR_REPO_BRANCH
168+
$prDiff = "$lintOutput\prDiff.patch"
169+
git diff -U0 FETCH_HEAD...HEAD > $prDiff
170+
$flake8Config = "$lintSource\flake8.ini"
171+
$flake8Output = "$lintOutput\PR-Flake8.txt"
172+
type "$prDiff" | py -m flake8 --disable-noqa --diff --output-file="$flake8Output" --tee --config="$flake8Config"
173+
if($LastExitCode -ne 0) {
174+
$errorCode=$LastExitCode
175+
Add-AppveyorMessage "PR introduces Flake8 errors"
176+
}
177+
Push-AppveyorArtifact $flake8Output
178+
$junitXML = "$lintOutput\PR-Flake8.xml"
179+
py "$lintSource\createJunitReport.py" "$flake8Output" "$junitXML"
180+
Push-AppveyorArtifact $junitXML
181+
$wc = New-Object 'System.Net.WebClient'
182+
$wc.UploadFile("https://ci.appveyor.com/api/testresults/junit/$($env:APPVEYOR_JOB_ID)", $junitXML)
183+
if($errorCode -ne 0) { $host.SetShouldExit($errorCode) }
184+
}
160185
- ps: |
161186
$testOutput = (Resolve-Path .\testOutput\)
162187
$systemTestOutput = (Resolve-Path "$testOutput\system")
@@ -169,9 +194,10 @@ test_script:
169194
$wc.UploadFile("https://ci.appveyor.com/api/testresults/junit/$($env:APPVEYOR_JOB_ID)", (Resolve-Path "$systemTestOutput\systemTests.xml"))
170195
if($errorCode -ne 0) { $host.SetShouldExit($errorCode) }
171196
172-
artifacts:
173-
- path: output\*
174-
- path: output\*\*
197+
on_finish:
198+
- ps: |
199+
Get-ChildItem output\* | % { Push-AppveyorArtifact $_.FullName -FileName $_.Name }
200+
Get-ChildItem output\*\* | % { Push-AppveyorArtifact $_.FullName -FileName $_.Name }
175201
176202
deploy_script:
177203
- ps: |
@@ -180,9 +206,9 @@ deploy_script:
180206
# Notify our server.
181207
$exe = Get-ChildItem -Name output\*.exe
182208
$hash = (Get-FileHash "output\$exe" -Algorithm SHA1).Hash.ToLower()
183-
$apiVersion = (python -c "import sys; sys.path.append('source'); from addonAPIVersion import CURRENT; print('{}.{}.{}'.format(*CURRENT))")
209+
$apiVersion = (py -c "import sys; sys.path.append('source'); from addonAPIVersion import CURRENT; print('{}.{}.{}'.format(*CURRENT))")
184210
echo apiversion: $apiVersion
185-
$apiCompatTo = (python -c "import sys; sys.path.append('source'); from addonAPIVersion import BACK_COMPAT_TO; print('{}.{}.{}'.format(*BACK_COMPAT_TO))")
211+
$apiCompatTo = (py -c "import sys; sys.path.append('source'); from addonAPIVersion import BACK_COMPAT_TO; print('{}.{}.{}'.format(*BACK_COMPAT_TO))")
186212
echo apiBackCompatTo: $apiCompatTo
187213
$data = @{
188214
jobId=$env:APPVEYOR_JOB_ID;

readme.md

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,13 @@ Additionally, the following build time dependencies are included in Git submodul
9090
* [Boost Optional (stand-alone header)](https://github.com/akrzemi1/Optional), from commit [3922965](https://github.com/akrzemi1/Optional/commit/3922965396fc455c6b1770374b9b4111799588a9)
9191

9292
### Other Dependencies
93-
These dependencies are not included in Git submodules, but aren't needed by most people.
93+
To lint using Flake 8 locally using our SCons integration, some dependencies are installed (automatically) via pip.
94+
Although this [must be run manually](#linting-your-changes), developers may wish to first configure a Python Virtual Environment to ensure their general install is not affected.
95+
* Flake8
96+
* Flake8-tabs
97+
98+
99+
The following dependencies aren't needed by most people, and are not included in Git submodules:
94100

95101
* To generate developer documentation for nvdaHelper: [Doxygen Windows installer](http://www.doxygen.nl/download.html), version 1.8.15:
96102

@@ -154,6 +160,8 @@ scons dist
154160

155161
The build will be created in the dist directory.
156162

163+
### Building the installer
164+
157165
To create a launcher archive (one executable allowing for installation or portable dist generation), type:
158166

159167
```
@@ -162,6 +170,8 @@ scons launcher
162170

163171
The archive will be placed in the output directory.
164172

173+
### Building the developer documentation
174+
165175
To generate the NVDA developer guide, type:
166176

167177
```
@@ -179,6 +189,7 @@ scons devDocs_nvdaHelper
179189

180190
The documentation will be placed in the `devDocs\nvdaHelper` folder in the output directory.
181191

192+
### Generate debug symbols archive
182193
To generate an archive of debug symbols for the various dll/exe binaries, type:
183194

184195
```
@@ -187,12 +198,14 @@ scons symbolsArchive
187198

188199
The archive will be placed in the output directory.
189200

201+
### Generate translation template
190202
To generate a gettext translation template (for translators), type:
191203

192204
```
193205
scons pot
194206
```
195207

208+
### Customising the build
196209
Optionally, the build can be customised by providing variables on the command line:
197210

198211
* version: The version of this build.
@@ -215,15 +228,15 @@ scons launcher version=test1
215228
## Running Automated Tests
216229
If you make a change to the NVDA code, you should run NVDA's automated tests.
217230
These tests help to ensure that code changes do not unintentionally break functionality that was previously working.
218-
Currently, NVDA has two kinds of automated testing: unit tests and translatable string checks.
219231

220-
To run the tests, first change directory to the root of the NVDA source distribution as above.
232+
To run the tests (unit tests, translatable string checks), first change directory to the root of the NVDA source distribution as above.
221233
Then, run:
222234

223235
```
224236
scons tests
225237
```
226238

239+
### Unit tests
227240
To run only specific unit tests, specify them using the `unitTests` variable on the command line.
228241
The tests should be provided as a comma separated list.
229242
Each test should be specified as a Python module, class or method relative to the `tests\unit` directory.
@@ -233,12 +246,31 @@ For example, to run only methods in the `TestMove` and `TestSelection` classes i
233246
scons tests unitTests=test_cursorManager.TestMove,test_cursorManager.TestSelection
234247
```
235248

249+
### Translatable string checks
236250
To run only the translatable string checks (which check that all translatable strings have translator comments), run:
237251

238252
```
239253
scons checkPot
240254
```
241255

256+
### Linting your changes
257+
In order to ensure your changes comply with NVDA's coding style you can run the Flake8 linter locally.
258+
Running via SCons will use Flake8 to inspect only the differences between your working directory and the specified `base` branch.
259+
If you create a Pull Request, the `base` branch you use here should be the same as the target you would use for a Pull Request. In most cases it will be `origin/master`.
260+
```
261+
scons lint base=origin/master
262+
```
263+
264+
To be warned about linting errors faster, you may wish to integrate Flake8 other development tools you are using.
265+
Consider using the following command line:
266+
```
267+
git diff -U0 baseRef...HEAD | py -3 -m flake8 --diff --config='tests/lint/flake8.ini
268+
```
269+
Where:
270+
- `baseRef` is the branch your changes are targeting
271+
- `tests\lint\flake8.ini` is the path to the NVDA Flake8 configuration file
272+
273+
### System Tests
242274
You may also use scons to run the system tests, though this will still rely on having set up the dependencies (see `tests/system/readme.md`).
243275

244276
```

sconstruct

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,17 @@ vars.Add(ListVariable("nvdaHelperDebugFlags", "a list of debugging features you
7676
vars.Add(EnumVariable('nvdaHelperLogLevel','The level of logging you wish to see, lower is more verbose','15',allowed_values=[str(x) for x in range(60)]))
7777
if "tests" in COMMAND_LINE_TARGETS:
7878
vars.Add("unitTests", "A list of unit tests to run", "")
79+
if "lint" in COMMAND_LINE_TARGETS:
80+
vars.Add( # pass through variable that lint is requested
81+
"doLint",
82+
"internal use",
83+
True
84+
)
85+
vars.Add(
86+
"base",
87+
"Lint is done only on a diff, specify the ref to use as base for the diff.",
88+
None
89+
)
7990
if "systemTests" in COMMAND_LINE_TARGETS:
8091
vars.Add("filter", "A filter for the name of the system test(s) to run. Wildcards accepted.", "")
8192

test.py

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
import sys # noqa: F401 'sys' imported but unused
2+
import os
3+
from collections import * # noqa: F403 'from collections import *' used; unable to detect undefined names
4+
5+
d = {
6+
"answer": 42, # noqa: F601 dictionary key 'answer' repeated with different values
7+
"hello": "world",
8+
"answer": "new", # noqa: F601 dictionary key 'answer' repeated with different values
9+
}
10+
this = 8
11+
os = "hello" # noqa: F811 redefinition of unused 'os' from line 2
12+
thisThat = this+9 # noqa: E226 missing white space
13+
print(thisThat)
14+
15+
assert (False, "explain") # noqa: F631 assertion is always true, perhaps remove parentheses?
16+
17+
def thisFunc(inArg) -> str: # noqa: E302 expected 2 blank lines, found 1
18+
print(inArg) # blah # noqa: E261 at least two spaces before inline comment
19+
return 5 #blah# noqa: E262 inline comment should start with '# '
20+
21+
22+
# ET126 (flake8-tabs) unexpected number of tabs at start of definition line (expected 2, got 3)
23+
def thatFunc(
24+
inArgOverIndented # noqa: ET126
25+
) -> str:
26+
# F405 'unknownArg' may be undefined, or defined from star imports: collections
27+
# F821 undefined name 'unknownArg'
28+
print(unknownArg) # noqa: F821, F405
29+
return 5
30+
31+
32+
# ET126 (flake8-tabs) unexpected number of tabs at start of definition line (expected 2, got 3)
33+
# F811 redefinition of unused 'thatFunc' from line 13
34+
def thatFunc( # noqa: F811
35+
inArgUnderIndented # noqa: ET126
36+
) -> str:
37+
pass
38+
39+
40+
# ET126 (flake8-tabs) unexpected number of tabs at start of definition line (expected 2, got 3)
41+
def another(
42+
argIndents,
43+
dontMatch # noqa: ET126
44+
) -> float:
45+
somethingElse = 6 # noqa: F841 local variable 'somethingElse' is assigned to but never used
46+
print(argIndents)
47+
48+
49+
def extremlyLongFunctionName_blahblahblahblahblahblahblahblahblahblahblahlblahblahblahblahblahblahblahblahblahblahblahblahlblah(arg): # noqa: E501 line too long (133 > 110 characters)
50+
raise NotImplemented # noqa: F901 'raise NotImplemented' should be 'raise NotImplementedError'
51+
# noqa: W292 no newline at end of file

tests/lint/.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
current.diff
2+
current.lint

tests/lint/createJunitReport.py

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
# -*- coding: UTF-8 -*-
2+
# A part of NonVisual Desktop Access (NVDA)
3+
# This file is covered by the GNU General Public License.
4+
# See the file COPYING for more details.
5+
# Copyright (C) 2019 NV Access Limited
6+
import os
7+
from sys import argv
8+
9+
NO_ERROR = r'''<?xml version="1.0" encoding="UTF-8"?>
10+
<testsuite name="flake8" tests="1" errors="0" failures="0" skip="0">
11+
<testcase classname="flake8.lint" name="flake8_diff_lint" time="1.00">
12+
</testcase>
13+
</testsuite>
14+
'''
15+
16+
# With Error:
17+
WE_PRE = r'''<?xml version="1.0" encoding="UTF-8"?>
18+
<testsuite name="flake8" tests="1" errors="1" failures="0" skip="0">
19+
<testcase classname="flake8.lint" name="flake8_diff_lint" time="1.00">
20+
<error type="lintError" message="Linting errors occurred">
21+
<![CDATA[
22+
'''
23+
WE_POST = r'''
24+
]]>
25+
</error>
26+
</testcase>
27+
</testsuite>
28+
'''
29+
30+
31+
def makeJunitXML(inFileName, outFileName):
32+
with open(inFileName, 'rt', encoding='UTF-8') as flake8In:
33+
errorText = flake8In.read()
34+
if len(errorText) > 0:
35+
# make "with error" xml content
36+
outContents = f'{WE_PRE}{errorText}{WE_POST}'
37+
else:
38+
# make "no error" xml content
39+
outContents = NO_ERROR
40+
41+
with open(outFileName, 'wt', encoding='UTF-8') as out:
42+
out.write(outContents)
43+
44+
45+
def main():
46+
try:
47+
if len(argv) != 3:
48+
raise RuntimeError(
49+
f"{argv[0]} expects two arguments: flake8_output_file_name junit_file_name"
50+
)
51+
scriptName, flake8OutputFileName, junitFileName = argv
52+
if not os.path.isfile(flake8OutputFileName):
53+
raise RuntimeError(
54+
f"Flake8_output_file does not exist at {flake8OutputFileName}"
55+
)
56+
makeJunitXML(flake8OutputFileName, junitFileName)
57+
except Exception as e:
58+
print(e)
59+
raise e
60+
61+
62+
if __name__ == "__main__":
63+
# execute only if run as a script
64+
main()

tests/lint/flake8.ini

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
[flake8]
2+
3+
# Plugins
4+
use-flake8-tabs = True
5+
use-pycodestyle-indent = True
6+
7+
# Reporting
8+
statistics = True
9+
doctests = True
10+
show-source = True
11+
12+
# Options
13+
max-complexity = 15
14+
max-line-length = 110
15+
hang-closing = True
16+
17+
ignore =
18+
W191, # indentation contains tabs
19+
E126, # continuation line over-indented for hanging indent
20+
E133, # closing bracket is missing indentation
21+
22+
builtins = # inform flake8 about functions we consider built-in.
23+
_, # translation lookup
24+
pgettext, # translation lookup
25+
26+
exclude = # don't bother looking in the following subdirectories / files.
27+
.git,
28+
__pycache__,
29+
.tox,
30+
build,
31+
output,
32+
include,
33+
miscDeps,
34+
source/louis,

tests/lint/lintInstall/.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
_executed_requirements.txt
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
####### requirements.txt #######
2+
#
3+
###### Requirements for automated lint ######
4+
flake8 == 3.7.7
5+
flake8-tabs == 2.0.0

0 commit comments

Comments
 (0)