Commit Graph

5 Commits

Author SHA1 Message Date
Jun Wu
2698cf1d22 scripts: unify spwaning run-test logic
Summary:
Previously, lint.py and unit.py have different logic spawning
run-tests.py.

The logic in `unit.py` is more robust: it sets `PYTHONPATH` and
`cwd`. It sets `-j` according to CPU cores. It can find `run-tests.py`
even if `MERCURIALRUNTESTS` is not set. You can run the script
from any directory (not only reporoot).

Test Plan: Run `lint.py` and `unit.py`, from the `scripts` directory and reporoot.

Reviewers: #sourcecontrol, stash

Reviewed By: stash

Subscribers: stash, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D4095437

Signature: t1:4095437:1477663516:13a7ac4270435c272077132915f9d02cc98a5afb
2016-10-28 13:58:48 +01:00
Jun Wu
781aea8725 arc: do not run lint on unrelated files
Summary:
"arc lint" runs the lint script for each file in parallel:

  >>> [5, pid=27257] <exec> $ scripts/lint.py 'scripts/lint.py'
  >>> [6, pid=27257] <exec> $ scripts/lint.py 'scripts/unit.py'
  >>> [7, pid=27257] <exec> $ scripts/lint.py 'tests/test-check-code-hg.t'
  >>> [8, pid=27257] <exec> $ scripts/lint.py 'tests/test-check-pyflakes-hg.t'

That looks fine but `lint.py` runs `test-check*.t` which will check all
files. This is a typical `N^2` behavior and needs to be fixed.

This diff changes the linter and test code so the test only checks the
single selected file.

Test Plan:
Run `arc lint --trace` and check `lint.py` time usage. It went down from
10 seconds to 2 seconds for this diff.

Reviewers: #sourcecontrol, durham

Reviewed By: durham

Subscribers: mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D4051080

Signature: t1:4051080:1476985355:45845c6eb0e66cbfa000e61b7b496f2d00aeb042
2016-10-20 15:44:29 +01:00
Jun Wu
6e17356b2e arc: show pyflakes errors in lint
Summary: Pyflakes errors are worth looking.

Test Plan: `arc lint`

Reviewers: #sourcecontrol, durham

Reviewed By: durham

Subscribers: mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D4051066

Signature: t1:4051066:1476985284:378587efc0644fd395feb90f2580806364863012
2016-10-20 15:20:43 +01:00
Wez Furlong
3fdff8880a 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 10:56:47 -07:00
Wez Furlong
e71ecbab09 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 08:54:54 -07:00