fb-hgext: improve check code linter integration
Summary:
if you run `arc lint --output json` and don't have your environment correctly set up, the failure to run the linter was silently ignored.
I wanted to fix that, but then realized that we could integrate this better without too much effort, hence this diff.
This is a small python script that processes the output from the check code test and captures the appropriate context so that `arc lint` can show you where in the code the problem(s) lie.
Test Plan:
This is the output when the environment is not set up correctly:
```
$ arc lint
>>> Lint for lint.py:
Error (S&RX) ENVIRON
Please either set MERCURIALRUNTEST env var to the full path to
run-tests.py, or add the containing directory to your $PATH
>>> 1
Error (S&RX) ENVIRON
Please either set MERCURIALRUNTEST env var to the full path to
run-tests.py, or add the containing directory to your $PATH
>>> 1
```
This is the same thing but in json output mode:
```
$ arc lint --output json
{".arcconfig":[]}
{"scripts\/lint.py":[]}
{"lint.py":[{"line":"1","char":null,"code":"S&RX","severity":"error","name":"ENVIRON","description":"Please either set MERCURIALRUNTEST env var to the full path to run-tests.py, or add the containing directory to your $PATH","original":null,"replacement":null,"granularity":1,"locations":[],"bypassChangedLineFiltering":null,"context":""},{"line":"1","char":null,"code":"S&RX","severity":"error","name":"ENVIRON","description":"Please either set MERCURIALRUNTEST env var to the full path to run-tests.py, or add the containing directory to your $PATH","original":null,"replacement":null,"granularity":1,"locations":[],"bypassChangedLineFiltering":null,"context":""}]}
```
This shows the lint errors from my `hg publish` stack:
```
>>> Lint for phabricator/arcconfig.py:
Error (S&RX) CheckCode
don't raise generic exceptions
18 def load_for_path(path):
19 homedir = os.getenv('HOME')
20 if not homedir:
>>> 21 raise Exception('$HOME environment variable not found')
22
23 # Use their own file as a basis
24 userconfig = _load_file(os.path.join(homedir, '.arcrc')) or {}
```
Reviewers: #sourcecontrol, ttung, lcharignon
Reviewed By: lcharignon
Subscribers: mjpieters
Differential Revision: https://phabricator.fb.com/D3203666
Signature: t1:3203666:1461185075:e9b03c8251d7e3b8e965b81146d17cdeac75d837
2016-04-21 18:54:54 +03:00
|
|
|
#!/usr/bin/env python
|
|
|
|
import errno
|
|
|
|
import os
|
|
|
|
import subprocess
|
|
|
|
import re
|
|
|
|
import sys
|
|
|
|
|
|
|
|
""" Convert mercurial check-code errors into a format
|
|
|
|
that plays nicely with arc lint """
|
|
|
|
|
|
|
|
runner = os.environ.get('MERCURIALRUNTEST', 'run-tests.py')
|
|
|
|
|
|
|
|
if not os.path.exists(runner):
|
|
|
|
# If it looks like we're in facebook-hg-rpms, let's try
|
|
|
|
# running against the associated hg-crew tests
|
|
|
|
# otherwise, Popen will search for run-tests.py in the PATH
|
|
|
|
default_runner = os.path.relpath('../hg-crew/tests/run-tests.py')
|
|
|
|
if os.path.exists(default_runner):
|
|
|
|
runner = os.path.abspath(default_runner)
|
|
|
|
|
|
|
|
# Normalize the list of files that we should report on
|
|
|
|
wanted = set()
|
|
|
|
for path in sys.argv[1:]:
|
|
|
|
wanted.add(os.path.relpath(path))
|
|
|
|
|
2016-10-20 17:44:29 +03:00
|
|
|
# Export LINTFILES so tests can skip unrelated files
|
|
|
|
if wanted:
|
|
|
|
os.environ['LINTFILES'] = '\n'.join(sorted(wanted))
|
|
|
|
|
fb-hgext: improve check code linter integration
Summary:
if you run `arc lint --output json` and don't have your environment correctly set up, the failure to run the linter was silently ignored.
I wanted to fix that, but then realized that we could integrate this better without too much effort, hence this diff.
This is a small python script that processes the output from the check code test and captures the appropriate context so that `arc lint` can show you where in the code the problem(s) lie.
Test Plan:
This is the output when the environment is not set up correctly:
```
$ arc lint
>>> Lint for lint.py:
Error (S&RX) ENVIRON
Please either set MERCURIALRUNTEST env var to the full path to
run-tests.py, or add the containing directory to your $PATH
>>> 1
Error (S&RX) ENVIRON
Please either set MERCURIALRUNTEST env var to the full path to
run-tests.py, or add the containing directory to your $PATH
>>> 1
```
This is the same thing but in json output mode:
```
$ arc lint --output json
{".arcconfig":[]}
{"scripts\/lint.py":[]}
{"lint.py":[{"line":"1","char":null,"code":"S&RX","severity":"error","name":"ENVIRON","description":"Please either set MERCURIALRUNTEST env var to the full path to run-tests.py, or add the containing directory to your $PATH","original":null,"replacement":null,"granularity":1,"locations":[],"bypassChangedLineFiltering":null,"context":""},{"line":"1","char":null,"code":"S&RX","severity":"error","name":"ENVIRON","description":"Please either set MERCURIALRUNTEST env var to the full path to run-tests.py, or add the containing directory to your $PATH","original":null,"replacement":null,"granularity":1,"locations":[],"bypassChangedLineFiltering":null,"context":""}]}
```
This shows the lint errors from my `hg publish` stack:
```
>>> Lint for phabricator/arcconfig.py:
Error (S&RX) CheckCode
don't raise generic exceptions
18 def load_for_path(path):
19 homedir = os.getenv('HOME')
20 if not homedir:
>>> 21 raise Exception('$HOME environment variable not found')
22
23 # Use their own file as a basis
24 userconfig = _load_file(os.path.join(homedir, '.arcrc')) or {}
```
Reviewers: #sourcecontrol, ttung, lcharignon
Reviewed By: lcharignon
Subscribers: mjpieters
Differential Revision: https://phabricator.fb.com/D3203666
Signature: t1:3203666:1461185075:e9b03c8251d7e3b8e965b81146d17cdeac75d837
2016-04-21 18:54:54 +03:00
|
|
|
try:
|
2016-10-20 17:20:43 +03:00
|
|
|
args = [runner, '-j2', '-l',
|
|
|
|
'test-check-code-hg.t',
|
|
|
|
'test-check-pyflakes-hg.t']
|
fb-hgext: improve check code linter integration
Summary:
if you run `arc lint --output json` and don't have your environment correctly set up, the failure to run the linter was silently ignored.
I wanted to fix that, but then realized that we could integrate this better without too much effort, hence this diff.
This is a small python script that processes the output from the check code test and captures the appropriate context so that `arc lint` can show you where in the code the problem(s) lie.
Test Plan:
This is the output when the environment is not set up correctly:
```
$ arc lint
>>> Lint for lint.py:
Error (S&RX) ENVIRON
Please either set MERCURIALRUNTEST env var to the full path to
run-tests.py, or add the containing directory to your $PATH
>>> 1
Error (S&RX) ENVIRON
Please either set MERCURIALRUNTEST env var to the full path to
run-tests.py, or add the containing directory to your $PATH
>>> 1
```
This is the same thing but in json output mode:
```
$ arc lint --output json
{".arcconfig":[]}
{"scripts\/lint.py":[]}
{"lint.py":[{"line":"1","char":null,"code":"S&RX","severity":"error","name":"ENVIRON","description":"Please either set MERCURIALRUNTEST env var to the full path to run-tests.py, or add the containing directory to your $PATH","original":null,"replacement":null,"granularity":1,"locations":[],"bypassChangedLineFiltering":null,"context":""},{"line":"1","char":null,"code":"S&RX","severity":"error","name":"ENVIRON","description":"Please either set MERCURIALRUNTEST env var to the full path to run-tests.py, or add the containing directory to your $PATH","original":null,"replacement":null,"granularity":1,"locations":[],"bypassChangedLineFiltering":null,"context":""}]}
```
This shows the lint errors from my `hg publish` stack:
```
>>> Lint for phabricator/arcconfig.py:
Error (S&RX) CheckCode
don't raise generic exceptions
18 def load_for_path(path):
19 homedir = os.getenv('HOME')
20 if not homedir:
>>> 21 raise Exception('$HOME environment variable not found')
22
23 # Use their own file as a basis
24 userconfig = _load_file(os.path.join(homedir, '.arcrc')) or {}
```
Reviewers: #sourcecontrol, ttung, lcharignon
Reviewed By: lcharignon
Subscribers: mjpieters
Differential Revision: https://phabricator.fb.com/D3203666
Signature: t1:3203666:1461185075:e9b03c8251d7e3b8e965b81146d17cdeac75d837
2016-04-21 18:54:54 +03:00
|
|
|
|
|
|
|
# Check lz4revlog requirement
|
|
|
|
reporoot = os.path.join(os.path.dirname(os.path.dirname(__file__)), '.hg')
|
|
|
|
with open(os.path.join(reporoot, 'requires'), 'r') as f:
|
|
|
|
if 'lz4revlog\n' in f:
|
|
|
|
args.append('--extra-config-opt=extensions.lz4revlog=')
|
|
|
|
|
|
|
|
proc = subprocess.Popen(args, stdout=subprocess.PIPE,
|
|
|
|
stderr=subprocess.PIPE, cwd='tests')
|
|
|
|
except OSError as ex:
|
|
|
|
if ex.errno == errno.ENOENT:
|
|
|
|
print 'lint.py:1: ERROR:ENVIRON: Please either set ' + \
|
|
|
|
'MERCURIALRUNTEST var to the full path to run-tests.py, ' + \
|
|
|
|
'or add the containing directory to your $PATH'
|
|
|
|
else:
|
|
|
|
print 'lint.py:1: ERROR:OSError: %s: %s' % (runner, str(ex))
|
|
|
|
sys.exit(0)
|
|
|
|
|
|
|
|
output, error = proc.communicate()
|
|
|
|
|
fb-hgext: fix arc lint to catch the issue in D3236640
Summary:
we weren't matching this pattern:
```
$ ./scripts/lint.py
+++ /data/users/wez/facebook-hg-rpms/fb-hgext/tests/test-check-code-hg.t.err
@@ -37,6 +37,8 @@
Skipping fastmanifest/tree_copy_test.c it has no-che?k-code (glob)
Skipping fastmanifest/tree_diff.c it has no-che?k-code (glob)
Skipping fastmanifest/tree_diff_test.c it has no-che?k-code (glob)
+ Skipping fastmanifest/tree_disk.c it has no-che?k-code (glob)
+ Skipping fastmanifest/tree_disk_test.c it has no-che?k-code (glob)
Skipping fastmanifest/tree_iterate_rt.c it has no-che?k-code (glob)
Skipping fastmanifest/tree_iterator.c it has no-che?k-code (glob)
Skipping fastmanifest/tree_iterator.h it has no-che?k-code (glob)
ERROR: test-check-code-hg.t output changed
!
Failed test-check-code-hg.t: output changed
# Ran 1 tests, 0 skipped, 0 warned, 1 failed.
python hash seed: 583073521
```
Due to the way that arc lint works, we have to associate the linter issue
with the files that changed in the diff, but we don't have an easy way to
figure out the line number where the `no-check-code` line was added, so
we just use line number 0.
Test Plan:
`arc patch D3236640` and `arc lint`:
```
$ arc lint --everything
>>> Lint for fastmanifest/tree_disk.c:
Error (S&RX) CheckCode
Update tests/test-check-code-hg.t to add Skipping
fastmanifest/tree_disk.c it has no-che?k-code (glob)
>>> 1 // Copyright 2016-present Facebook. All Rights Reserved.
2 //
3 // tree_disk.c: methods to persist to and restore from disk.
4 //
>>> Lint for fastmanifest/tree_disk_test.c:
Error (S&RX) CheckCode
Update tests/test-check-code-hg.t to add Skipping
fastmanifest/tree_disk_test.c it has no-che?k-code (glob)
>>> 1 // Copyright 2016-present Facebook. All Rights Reserved.
2 //
3 // tree_disk_test.c: tests to verify tree_disk
4 //
```
Reviewers: #sourcecontrol, ttung, lcharignon
Reviewed By: lcharignon
Subscribers: lcharignon, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3237054
Signature: t1:3237054:1461949072:ac7cc29fcee9dbc00a4019088f3adb50de9625bf
2016-04-29 20:56:47 +03:00
|
|
|
context_file = None
|
fb-hgext: improve check code linter integration
Summary:
if you run `arc lint --output json` and don't have your environment correctly set up, the failure to run the linter was silently ignored.
I wanted to fix that, but then realized that we could integrate this better without too much effort, hence this diff.
This is a small python script that processes the output from the check code test and captures the appropriate context so that `arc lint` can show you where in the code the problem(s) lie.
Test Plan:
This is the output when the environment is not set up correctly:
```
$ arc lint
>>> Lint for lint.py:
Error (S&RX) ENVIRON
Please either set MERCURIALRUNTEST env var to the full path to
run-tests.py, or add the containing directory to your $PATH
>>> 1
Error (S&RX) ENVIRON
Please either set MERCURIALRUNTEST env var to the full path to
run-tests.py, or add the containing directory to your $PATH
>>> 1
```
This is the same thing but in json output mode:
```
$ arc lint --output json
{".arcconfig":[]}
{"scripts\/lint.py":[]}
{"lint.py":[{"line":"1","char":null,"code":"S&RX","severity":"error","name":"ENVIRON","description":"Please either set MERCURIALRUNTEST env var to the full path to run-tests.py, or add the containing directory to your $PATH","original":null,"replacement":null,"granularity":1,"locations":[],"bypassChangedLineFiltering":null,"context":""},{"line":"1","char":null,"code":"S&RX","severity":"error","name":"ENVIRON","description":"Please either set MERCURIALRUNTEST env var to the full path to run-tests.py, or add the containing directory to your $PATH","original":null,"replacement":null,"granularity":1,"locations":[],"bypassChangedLineFiltering":null,"context":""}]}
```
This shows the lint errors from my `hg publish` stack:
```
>>> Lint for phabricator/arcconfig.py:
Error (S&RX) CheckCode
don't raise generic exceptions
18 def load_for_path(path):
19 homedir = os.getenv('HOME')
20 if not homedir:
>>> 21 raise Exception('$HOME environment variable not found')
22
23 # Use their own file as a basis
24 userconfig = _load_file(os.path.join(homedir, '.arcrc')) or {}
```
Reviewers: #sourcecontrol, ttung, lcharignon
Reviewed By: lcharignon
Subscribers: mjpieters
Differential Revision: https://phabricator.fb.com/D3203666
Signature: t1:3203666:1461185075:e9b03c8251d7e3b8e965b81146d17cdeac75d837
2016-04-21 18:54:54 +03:00
|
|
|
lines = error.split('\n')
|
|
|
|
# We expect a run of 3 lines to describe the error, with the first
|
|
|
|
# of those to look like a filename and line number location
|
fb-hgext: fix arc lint to catch the issue in D3236640
Summary:
we weren't matching this pattern:
```
$ ./scripts/lint.py
+++ /data/users/wez/facebook-hg-rpms/fb-hgext/tests/test-check-code-hg.t.err
@@ -37,6 +37,8 @@
Skipping fastmanifest/tree_copy_test.c it has no-che?k-code (glob)
Skipping fastmanifest/tree_diff.c it has no-che?k-code (glob)
Skipping fastmanifest/tree_diff_test.c it has no-che?k-code (glob)
+ Skipping fastmanifest/tree_disk.c it has no-che?k-code (glob)
+ Skipping fastmanifest/tree_disk_test.c it has no-che?k-code (glob)
Skipping fastmanifest/tree_iterate_rt.c it has no-che?k-code (glob)
Skipping fastmanifest/tree_iterator.c it has no-che?k-code (glob)
Skipping fastmanifest/tree_iterator.h it has no-che?k-code (glob)
ERROR: test-check-code-hg.t output changed
!
Failed test-check-code-hg.t: output changed
# Ran 1 tests, 0 skipped, 0 warned, 1 failed.
python hash seed: 583073521
```
Due to the way that arc lint works, we have to associate the linter issue
with the files that changed in the diff, but we don't have an easy way to
figure out the line number where the `no-check-code` line was added, so
we just use line number 0.
Test Plan:
`arc patch D3236640` and `arc lint`:
```
$ arc lint --everything
>>> Lint for fastmanifest/tree_disk.c:
Error (S&RX) CheckCode
Update tests/test-check-code-hg.t to add Skipping
fastmanifest/tree_disk.c it has no-che?k-code (glob)
>>> 1 // Copyright 2016-present Facebook. All Rights Reserved.
2 //
3 // tree_disk.c: methods to persist to and restore from disk.
4 //
>>> Lint for fastmanifest/tree_disk_test.c:
Error (S&RX) CheckCode
Update tests/test-check-code-hg.t to add Skipping
fastmanifest/tree_disk_test.c it has no-che?k-code (glob)
>>> 1 // Copyright 2016-present Facebook. All Rights Reserved.
2 //
3 // tree_disk_test.c: tests to verify tree_disk
4 //
```
Reviewers: #sourcecontrol, ttung, lcharignon
Reviewed By: lcharignon
Subscribers: lcharignon, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3237054
Signature: t1:3237054:1461949072:ac7cc29fcee9dbc00a4019088f3adb50de9625bf
2016-04-29 20:56:47 +03:00
|
|
|
while lines:
|
fb-hgext: improve check code linter integration
Summary:
if you run `arc lint --output json` and don't have your environment correctly set up, the failure to run the linter was silently ignored.
I wanted to fix that, but then realized that we could integrate this better without too much effort, hence this diff.
This is a small python script that processes the output from the check code test and captures the appropriate context so that `arc lint` can show you where in the code the problem(s) lie.
Test Plan:
This is the output when the environment is not set up correctly:
```
$ arc lint
>>> Lint for lint.py:
Error (S&RX) ENVIRON
Please either set MERCURIALRUNTEST env var to the full path to
run-tests.py, or add the containing directory to your $PATH
>>> 1
Error (S&RX) ENVIRON
Please either set MERCURIALRUNTEST env var to the full path to
run-tests.py, or add the containing directory to your $PATH
>>> 1
```
This is the same thing but in json output mode:
```
$ arc lint --output json
{".arcconfig":[]}
{"scripts\/lint.py":[]}
{"lint.py":[{"line":"1","char":null,"code":"S&RX","severity":"error","name":"ENVIRON","description":"Please either set MERCURIALRUNTEST env var to the full path to run-tests.py, or add the containing directory to your $PATH","original":null,"replacement":null,"granularity":1,"locations":[],"bypassChangedLineFiltering":null,"context":""},{"line":"1","char":null,"code":"S&RX","severity":"error","name":"ENVIRON","description":"Please either set MERCURIALRUNTEST env var to the full path to run-tests.py, or add the containing directory to your $PATH","original":null,"replacement":null,"granularity":1,"locations":[],"bypassChangedLineFiltering":null,"context":""}]}
```
This shows the lint errors from my `hg publish` stack:
```
>>> Lint for phabricator/arcconfig.py:
Error (S&RX) CheckCode
don't raise generic exceptions
18 def load_for_path(path):
19 homedir = os.getenv('HOME')
20 if not homedir:
>>> 21 raise Exception('$HOME environment variable not found')
22
23 # Use their own file as a basis
24 userconfig = _load_file(os.path.join(homedir, '.arcrc')) or {}
```
Reviewers: #sourcecontrol, ttung, lcharignon
Reviewed By: lcharignon
Subscribers: mjpieters
Differential Revision: https://phabricator.fb.com/D3203666
Signature: t1:3203666:1461185075:e9b03c8251d7e3b8e965b81146d17cdeac75d837
2016-04-21 18:54:54 +03:00
|
|
|
line = lines[0]
|
fb-hgext: fix arc lint to catch the issue in D3236640
Summary:
we weren't matching this pattern:
```
$ ./scripts/lint.py
+++ /data/users/wez/facebook-hg-rpms/fb-hgext/tests/test-check-code-hg.t.err
@@ -37,6 +37,8 @@
Skipping fastmanifest/tree_copy_test.c it has no-che?k-code (glob)
Skipping fastmanifest/tree_diff.c it has no-che?k-code (glob)
Skipping fastmanifest/tree_diff_test.c it has no-che?k-code (glob)
+ Skipping fastmanifest/tree_disk.c it has no-che?k-code (glob)
+ Skipping fastmanifest/tree_disk_test.c it has no-che?k-code (glob)
Skipping fastmanifest/tree_iterate_rt.c it has no-che?k-code (glob)
Skipping fastmanifest/tree_iterator.c it has no-che?k-code (glob)
Skipping fastmanifest/tree_iterator.h it has no-che?k-code (glob)
ERROR: test-check-code-hg.t output changed
!
Failed test-check-code-hg.t: output changed
# Ran 1 tests, 0 skipped, 0 warned, 1 failed.
python hash seed: 583073521
```
Due to the way that arc lint works, we have to associate the linter issue
with the files that changed in the diff, but we don't have an easy way to
figure out the line number where the `no-check-code` line was added, so
we just use line number 0.
Test Plan:
`arc patch D3236640` and `arc lint`:
```
$ arc lint --everything
>>> Lint for fastmanifest/tree_disk.c:
Error (S&RX) CheckCode
Update tests/test-check-code-hg.t to add Skipping
fastmanifest/tree_disk.c it has no-che?k-code (glob)
>>> 1 // Copyright 2016-present Facebook. All Rights Reserved.
2 //
3 // tree_disk.c: methods to persist to and restore from disk.
4 //
>>> Lint for fastmanifest/tree_disk_test.c:
Error (S&RX) CheckCode
Update tests/test-check-code-hg.t to add Skipping
fastmanifest/tree_disk_test.c it has no-che?k-code (glob)
>>> 1 // Copyright 2016-present Facebook. All Rights Reserved.
2 //
3 // tree_disk_test.c: tests to verify tree_disk
4 //
```
Reviewers: #sourcecontrol, ttung, lcharignon
Reviewed By: lcharignon
Subscribers: lcharignon, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3237054
Signature: t1:3237054:1461949072:ac7cc29fcee9dbc00a4019088f3adb50de9625bf
2016-04-29 20:56:47 +03:00
|
|
|
lines.pop(0)
|
|
|
|
|
2016-10-20 17:20:43 +03:00
|
|
|
# test-check-pyflakes-hg style output
|
|
|
|
m = re.match('^\+ ([a-zA-Z0-9_./-]+):(\d+): (.*)$', line)
|
|
|
|
if m:
|
|
|
|
filename, location, why = m.groups()
|
|
|
|
if filename in wanted:
|
|
|
|
print '%s:%s: ERROR:Pyflakes: %s' % (filename, location, why)
|
|
|
|
continue
|
|
|
|
|
|
|
|
# test-check-code-hg style output
|
fb-hgext: fix arc lint to catch the issue in D3236640
Summary:
we weren't matching this pattern:
```
$ ./scripts/lint.py
+++ /data/users/wez/facebook-hg-rpms/fb-hgext/tests/test-check-code-hg.t.err
@@ -37,6 +37,8 @@
Skipping fastmanifest/tree_copy_test.c it has no-che?k-code (glob)
Skipping fastmanifest/tree_diff.c it has no-che?k-code (glob)
Skipping fastmanifest/tree_diff_test.c it has no-che?k-code (glob)
+ Skipping fastmanifest/tree_disk.c it has no-che?k-code (glob)
+ Skipping fastmanifest/tree_disk_test.c it has no-che?k-code (glob)
Skipping fastmanifest/tree_iterate_rt.c it has no-che?k-code (glob)
Skipping fastmanifest/tree_iterator.c it has no-che?k-code (glob)
Skipping fastmanifest/tree_iterator.h it has no-che?k-code (glob)
ERROR: test-check-code-hg.t output changed
!
Failed test-check-code-hg.t: output changed
# Ran 1 tests, 0 skipped, 0 warned, 1 failed.
python hash seed: 583073521
```
Due to the way that arc lint works, we have to associate the linter issue
with the files that changed in the diff, but we don't have an easy way to
figure out the line number where the `no-check-code` line was added, so
we just use line number 0.
Test Plan:
`arc patch D3236640` and `arc lint`:
```
$ arc lint --everything
>>> Lint for fastmanifest/tree_disk.c:
Error (S&RX) CheckCode
Update tests/test-check-code-hg.t to add Skipping
fastmanifest/tree_disk.c it has no-che?k-code (glob)
>>> 1 // Copyright 2016-present Facebook. All Rights Reserved.
2 //
3 // tree_disk.c: methods to persist to and restore from disk.
4 //
>>> Lint for fastmanifest/tree_disk_test.c:
Error (S&RX) CheckCode
Update tests/test-check-code-hg.t to add Skipping
fastmanifest/tree_disk_test.c it has no-che?k-code (glob)
>>> 1 // Copyright 2016-present Facebook. All Rights Reserved.
2 //
3 // tree_disk_test.c: tests to verify tree_disk
4 //
```
Reviewers: #sourcecontrol, ttung, lcharignon
Reviewed By: lcharignon
Subscribers: lcharignon, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3237054
Signature: t1:3237054:1461949072:ac7cc29fcee9dbc00a4019088f3adb50de9625bf
2016-04-29 20:56:47 +03:00
|
|
|
if re.match('^--- (.*)$', line):
|
|
|
|
context_file = os.path.relpath(line[4:])
|
|
|
|
continue
|
|
|
|
|
|
|
|
m = re.match('^\+ (Skipping (.*) it has no.*)$', line)
|
|
|
|
if m:
|
|
|
|
filename = m.group(2)
|
|
|
|
if filename in wanted:
|
|
|
|
print '%s:0: ERROR:CheckCode: Update %s to add %s' % (
|
|
|
|
filename, context_file, m.group(1))
|
|
|
|
continue
|
|
|
|
|
fb-hgext: improve check code linter integration
Summary:
if you run `arc lint --output json` and don't have your environment correctly set up, the failure to run the linter was silently ignored.
I wanted to fix that, but then realized that we could integrate this better without too much effort, hence this diff.
This is a small python script that processes the output from the check code test and captures the appropriate context so that `arc lint` can show you where in the code the problem(s) lie.
Test Plan:
This is the output when the environment is not set up correctly:
```
$ arc lint
>>> Lint for lint.py:
Error (S&RX) ENVIRON
Please either set MERCURIALRUNTEST env var to the full path to
run-tests.py, or add the containing directory to your $PATH
>>> 1
Error (S&RX) ENVIRON
Please either set MERCURIALRUNTEST env var to the full path to
run-tests.py, or add the containing directory to your $PATH
>>> 1
```
This is the same thing but in json output mode:
```
$ arc lint --output json
{".arcconfig":[]}
{"scripts\/lint.py":[]}
{"lint.py":[{"line":"1","char":null,"code":"S&RX","severity":"error","name":"ENVIRON","description":"Please either set MERCURIALRUNTEST env var to the full path to run-tests.py, or add the containing directory to your $PATH","original":null,"replacement":null,"granularity":1,"locations":[],"bypassChangedLineFiltering":null,"context":""},{"line":"1","char":null,"code":"S&RX","severity":"error","name":"ENVIRON","description":"Please either set MERCURIALRUNTEST env var to the full path to run-tests.py, or add the containing directory to your $PATH","original":null,"replacement":null,"granularity":1,"locations":[],"bypassChangedLineFiltering":null,"context":""}]}
```
This shows the lint errors from my `hg publish` stack:
```
>>> Lint for phabricator/arcconfig.py:
Error (S&RX) CheckCode
don't raise generic exceptions
18 def load_for_path(path):
19 homedir = os.getenv('HOME')
20 if not homedir:
>>> 21 raise Exception('$HOME environment variable not found')
22
23 # Use their own file as a basis
24 userconfig = _load_file(os.path.join(homedir, '.arcrc')) or {}
```
Reviewers: #sourcecontrol, ttung, lcharignon
Reviewed By: lcharignon
Subscribers: mjpieters
Differential Revision: https://phabricator.fb.com/D3203666
Signature: t1:3203666:1461185075:e9b03c8251d7e3b8e965b81146d17cdeac75d837
2016-04-21 18:54:54 +03:00
|
|
|
if not re.match('^\+ +[a-zA-Z0-9_./-]+:\d+:$', line):
|
|
|
|
continue
|
fb-hgext: fix arc lint to catch the issue in D3236640
Summary:
we weren't matching this pattern:
```
$ ./scripts/lint.py
+++ /data/users/wez/facebook-hg-rpms/fb-hgext/tests/test-check-code-hg.t.err
@@ -37,6 +37,8 @@
Skipping fastmanifest/tree_copy_test.c it has no-che?k-code (glob)
Skipping fastmanifest/tree_diff.c it has no-che?k-code (glob)
Skipping fastmanifest/tree_diff_test.c it has no-che?k-code (glob)
+ Skipping fastmanifest/tree_disk.c it has no-che?k-code (glob)
+ Skipping fastmanifest/tree_disk_test.c it has no-che?k-code (glob)
Skipping fastmanifest/tree_iterate_rt.c it has no-che?k-code (glob)
Skipping fastmanifest/tree_iterator.c it has no-che?k-code (glob)
Skipping fastmanifest/tree_iterator.h it has no-che?k-code (glob)
ERROR: test-check-code-hg.t output changed
!
Failed test-check-code-hg.t: output changed
# Ran 1 tests, 0 skipped, 0 warned, 1 failed.
python hash seed: 583073521
```
Due to the way that arc lint works, we have to associate the linter issue
with the files that changed in the diff, but we don't have an easy way to
figure out the line number where the `no-check-code` line was added, so
we just use line number 0.
Test Plan:
`arc patch D3236640` and `arc lint`:
```
$ arc lint --everything
>>> Lint for fastmanifest/tree_disk.c:
Error (S&RX) CheckCode
Update tests/test-check-code-hg.t to add Skipping
fastmanifest/tree_disk.c it has no-che?k-code (glob)
>>> 1 // Copyright 2016-present Facebook. All Rights Reserved.
2 //
3 // tree_disk.c: methods to persist to and restore from disk.
4 //
>>> Lint for fastmanifest/tree_disk_test.c:
Error (S&RX) CheckCode
Update tests/test-check-code-hg.t to add Skipping
fastmanifest/tree_disk_test.c it has no-che?k-code (glob)
>>> 1 // Copyright 2016-present Facebook. All Rights Reserved.
2 //
3 // tree_disk_test.c: tests to verify tree_disk
4 //
```
Reviewers: #sourcecontrol, ttung, lcharignon
Reviewed By: lcharignon
Subscribers: lcharignon, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3237054
Signature: t1:3237054:1461949072:ac7cc29fcee9dbc00a4019088f3adb50de9625bf
2016-04-29 20:56:47 +03:00
|
|
|
|
|
|
|
if len(lines) < 2:
|
|
|
|
continue
|
|
|
|
|
|
|
|
location = line
|
|
|
|
context = lines.pop(0) # we ignore this
|
|
|
|
why = lines.pop(0)
|
fb-hgext: improve check code linter integration
Summary:
if you run `arc lint --output json` and don't have your environment correctly set up, the failure to run the linter was silently ignored.
I wanted to fix that, but then realized that we could integrate this better without too much effort, hence this diff.
This is a small python script that processes the output from the check code test and captures the appropriate context so that `arc lint` can show you where in the code the problem(s) lie.
Test Plan:
This is the output when the environment is not set up correctly:
```
$ arc lint
>>> Lint for lint.py:
Error (S&RX) ENVIRON
Please either set MERCURIALRUNTEST env var to the full path to
run-tests.py, or add the containing directory to your $PATH
>>> 1
Error (S&RX) ENVIRON
Please either set MERCURIALRUNTEST env var to the full path to
run-tests.py, or add the containing directory to your $PATH
>>> 1
```
This is the same thing but in json output mode:
```
$ arc lint --output json
{".arcconfig":[]}
{"scripts\/lint.py":[]}
{"lint.py":[{"line":"1","char":null,"code":"S&RX","severity":"error","name":"ENVIRON","description":"Please either set MERCURIALRUNTEST env var to the full path to run-tests.py, or add the containing directory to your $PATH","original":null,"replacement":null,"granularity":1,"locations":[],"bypassChangedLineFiltering":null,"context":""},{"line":"1","char":null,"code":"S&RX","severity":"error","name":"ENVIRON","description":"Please either set MERCURIALRUNTEST env var to the full path to run-tests.py, or add the containing directory to your $PATH","original":null,"replacement":null,"granularity":1,"locations":[],"bypassChangedLineFiltering":null,"context":""}]}
```
This shows the lint errors from my `hg publish` stack:
```
>>> Lint for phabricator/arcconfig.py:
Error (S&RX) CheckCode
don't raise generic exceptions
18 def load_for_path(path):
19 homedir = os.getenv('HOME')
20 if not homedir:
>>> 21 raise Exception('$HOME environment variable not found')
22
23 # Use their own file as a basis
24 userconfig = _load_file(os.path.join(homedir, '.arcrc')) or {}
```
Reviewers: #sourcecontrol, ttung, lcharignon
Reviewed By: lcharignon
Subscribers: mjpieters
Differential Revision: https://phabricator.fb.com/D3203666
Signature: t1:3203666:1461185075:e9b03c8251d7e3b8e965b81146d17cdeac75d837
2016-04-21 18:54:54 +03:00
|
|
|
|
|
|
|
location = location[1:].strip() # strip off the "+ " bit
|
|
|
|
location = location.rstrip(':')
|
|
|
|
filename, lineno = location.split(':')
|
|
|
|
|
|
|
|
if filename not in wanted:
|
|
|
|
# lint doesn't care about this file.
|
|
|
|
continue
|
|
|
|
|
|
|
|
why = why[1:].strip()
|
|
|
|
|
|
|
|
print '%s: ERROR:CheckCode: %s' % (location, why)
|
|
|
|
|