Age | Commit message (Collapse) | Author |
|
This CL rewrites our githooks in Python, since bash makes this
moderately ugly.
The Killer Feature(TM) here is that we now also try to execute the
commands that we recommend the user to run. This makes fixing many "yapf
disagrees with you" / "you're missing a #!"-style messages as simple as
`git commit --amend -a`.
Note that these fixes are only auto-applied if your current repo is
clean.
BUG=None
TEST=Various broken and configs in and outside of the chroot. I was also
able to `repo upload` with it. :)
Change-Id: Id841fa02271c0d03debbf35681eba8151074672e
Reviewed-on: https://chromium-review.googlesource.com/1897253
Tested-by: George Burgess <gbiv@chromium.org>
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Legacy-Commit-Queue: Commit Bot <commit-bot@chromium.org>
Reviewed-by: Denis Nikitin <denik@chromium.org>
|
|
This CL fixes a bug in the previous one editing pre-submit which caused
presubmit to fail when no new files have been added.
BUG=None
TEST=created new branches, confirmed presubmit is run correctly
Change-Id: I1a1b968220e60db12d05bbdab25646bfe2eb42e0
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/1710022
Reviewed-by: George Burgess <gbiv@chromium.org>
Reviewed-by: Caroline Tice <cmtice@chromium.org>
Tested-by: Emma Vukelj <emmavukelj@google.com>
|
|
This CL changes the presubmit hook to only lint files that exist, to
avoid trying to lint files that a CL removed and failing fatally.
BUG=None
TEST=created new branch, confirmed existing files are linted and
deleted files are not.
Change-Id: Ie812914fb2b33de75cda5a036df18750dcc6e6bf
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/1710018
Reviewed-by: Caroline Tice <cmtice@chromium.org>
Reviewed-by: George Burgess <gbiv@chromium.org>
Tested-by: Emma Vukelj <emmavukelj@google.com>
|
|
(via go test) during presubmit and git hooks.
Will only warn when the go commands are not available
as go is not part of depot_tools.
BUG=chromium:976903
TEST=Tried with and without go/gofmt/golint in the path.
TEST=Tried on go and non go files.
TEST=Tried on go files with and without errors.
Change-Id: I8a6aa4227f0d8649f7c390ab59f187f14955293c
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/1669745
Reviewed-by: George Burgess <gbiv@chromium.org>
Commit-Queue: Tobias Bosch <tbosch@google.com>
Tested-by: Tobias Bosch <tbosch@google.com>
Legacy-Commit-Queue: Commit Bot <commit-bot@chromium.org>
|
|
Also renames a few things for clarity.
BUG=None
TEST=repo upload
Change-Id: I7fec9cc90be7cea7dac69aae61942ebdd8868247
Reviewed-on: https://chromium-review.googlesource.com/1544903
Commit-Ready: George Burgess <gbiv@chromium.org>
Tested-by: George Burgess <gbiv@chromium.org>
Reviewed-by: George Burgess <gbiv@chromium.org>
|
|
This is three small changes in one.
The main attraction is no functional change. The benefits we see are:
A. less cluttered output
B. total wall time of test running is down from 18s to
13s on my machine, since the test runner spawns all
tests in parallel
The second change is that we now set PYTHONPATH for linting. Otherwise,
we'll see errors about bad imports.
Finally, this fixes a test. It appears that the parameters of this
assertRaises function have been accidentally flipped, though that works
without issue outside of the chroot. To understand why, consider that
assertRaises has two forms that act completely differently:
self.assertRaises(Exception, foo)
Which causes self.assertRaises to run foo(), and expect that foo()
raises an Exception.
Additionally, it can be used like so:
with self.assertRaises(Exception):
foo()
Which causes assertRaises to intercept any Exception that the block
under it may throw.
The difference between the in-chroot and out-of-chroot Python is simple:
outside of the chroot, the default value for the second arg to
assertRaises -- not counting `self` -- is None. Inside of the chroot,
they have a special, hidden sentinel value that it defaults to.
So, *outside* of the chroot, `self.assertRaises(Exception)` is
equivalent to `self.assertRaises(Exception, None)`, so the latter
returns a context object that tries to catch an exception.
Inside of the chroot, `self.assertRaises(Exception, None)` is not
equivalent to `self.assertRaises(Exception)`, and `None` is called as
though it was a function.
Since the removed code was `self.assertRaises(foo, None)`, outside of
the chroot, we'd just be returned a context object that we dropped on
the floor. Inside of the chroot, `self.assertRaises(foo, None)` would
try to call `None()`, fail, then try to say
`issubclass(exception_that_calling_none_raised, foo)`, but foo is a
function, so `issubclass` would raise on its own.
For those keeping score at home, the chroot's behavior was introduced in
upstream Python in 7f71e04cb510c24be337a22350324dc8a28e9775, which
landed in the 2.7.10 release. It wasn't until 2.7.11 that the revert
049060c249a83b69c4841ed37b7f4303f9ad7dd7 took effect. For more, the bug
was https://bugs.python.org/issue24134
BUG=None
TEST=./run_tests.sh
Change-Id: I58c398caafde03242ed3ca7530bf9a819fae99e2
Reviewed-on: https://chromium-review.googlesource.com/1548399
Commit-Ready: George Burgess <gbiv@chromium.org>
Tested-by: George Burgess <gbiv@chromium.org>
Reviewed-by: Caroline Tice <cmtice@chromium.org>
|
|
It sounds like we want to start using check-style as part of a test
runner. Refactoring check-style a bit allows us to trivially do that
cleanly in parallel with code style/lint checks.
No functional change is intended.
BUG=None
TEST=repo upload
Change-Id: I1fea43922dd37e571b7d2695656f480d5b458460
Reviewed-on: https://chromium-review.googlesource.com/1516417
Commit-Ready: George Burgess <gbiv@chromium.org>
Tested-by: George Burgess <gbiv@chromium.org>
Reviewed-by: Luis Lozano <llozano@chromium.org>
|
|
If all we're doing is deleting a file, this hook will fail loudly.
That's not what we want with ${PRESUBMIT_FILES}, since that's empty if
all we're doing is deleting files.
BUG=None
TEST=repo upload
Change-Id: I0a2e88c635f614ada113e2c29dd9509714680135
Reviewed-on: https://chromium-review.googlesource.com/1512954
Commit-Ready: George Burgess <gbiv@chromium.org>
Tested-by: George Burgess <gbiv@chromium.org>
Reviewed-by: Caroline Tice <cmtice@chromium.org>
|
|
We appear to have workflows and committers that prefer to `push` from
outside of the CrOS source tree.
It's relatively straightforward to have fallbacks to support that, so I
don't see why not.
If you want to interact with this repository from outside of a CrOS
source tree, you now have two options:
- If you have a source tree somewhere on your machine, you can set
CHROMEOS_ROOT_DIRECTORY to that path. If `check-lint` detects a usable
`cros` in there, it'll function as usual.
- If you don't have a source tree on your machine, `check-lint` will
fall back to pylint-only linting (with a warning). This isn't ideal
(we miss shell linting, as well as a few other nits in Python,
somehow), but should get us most of the way to happiness.
This also includes a refactor of `test` to `[[`
BUG=chromium:918755
TEST=Ran check-lint on some python files in a checkout outside and
inside of the source tree
Change-Id: I101c4c8b142f0c8f78819779adcfd0b3b0e1d614
Reviewed-on: https://chromium-review.googlesource.com/1412682
Commit-Ready: George Burgess <gbiv@chromium.org>
Tested-by: George Burgess <gbiv@chromium.org>
Reviewed-by: Caroline Tice <cmtice@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
|
|
Per the comment in I101c4c8b142f0c8f78819779adcfd0b3b0e1d614, `[[` is
preferable to `test` where possible. Swap to that for consistency.
(`[[` isn't an actual command, so we have to stick to `test` in
status_to_tf)
BUG=chromium:918755
TEST=check-style on a handful of Python files
Change-Id: Ic2256fadc570a5d3818918e49de45d641a21574a
Reviewed-on: https://chromium-review.googlesource.com/1415683
Commit-Ready: George Burgess <gbiv@chromium.org>
Tested-by: George Burgess <gbiv@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
|
|
This adds a git hook to run `cros lint` on all of the files we're
committing. It also adds a wrapper around both check-lint and
check-format that runs the two in parallel without making the output
interleave. Better names for check-style are appreciated.
BUG=chromium:918755
TEST=Ran ./check-style on a few Python files. Got the expected
output/lint errors/etc. Works both outside and inside of the
chroot.
CQ-DEPEND=CL:1394285
Change-Id: I30c74662759f27abdfd663083d04002a05260bf7
Reviewed-on: https://chromium-review.googlesource.com/1395807
Commit-Ready: George Burgess <gbiv@chromium.org>
Tested-by: George Burgess <gbiv@chromium.org>
Reviewed-by: Manoj Gupta <manojgupta@chromium.org>
|
|
This updates a few things:
- Migrates our git hooks to directly use yapf for formatting (which
appears to be the direction that Chromium is heading in, and is
available by default both inside and outside of the chroot: Chromium's
depot_tools has a copy of it. pyformat is only available internally,
and is basically a wrapper around yapf, except for a few things.
Please see CL 1392445 for more),
- Splits out the format checks into its own script, so it's easier to
potentially run from e.g. repo (don't quote me on that quite yet,
though ;) )
- Removes the now-seemingly-unused tc_format script
BUG=chromium:918755
TEST=- ./check-format on a poorly formatted Python file
- ./check-format on a python file with a #! that shouldn't have had
it
- ./check-format on a python file without a #! that should have had
it
- ./check-format on toolchain-utils/*.py; results were overall
reasonable
All fixes it suggested worked as intended.
CQ-DEPEND=CL:1392445
Change-Id: Idabc2658c5dedcc70b6d3cc6c1acfd6c83eabc29
Reviewed-on: https://chromium-review.googlesource.com/1394285
Commit-Ready: George Burgess <gbiv@chromium.org>
Tested-by: George Burgess <gbiv@chromium.org>
Reviewed-by: Manoj Gupta <manojgupta@chromium.org>
Reviewed-by: Luis Lozano <llozano@chromium.org>
|
|
Run pyformat before git push. Give error if files need formatting.
Also, made silly cleanup to pstat.py to be able to test the pre-push
hook.
BUG=None
TEST=white box testing
Change-Id: I79f12fa0acca4048b551f7dab25b0de8d5f05d75
Reviewed-on: https://chrome-internal-review.googlesource.com/243113
Commit-Ready: Luis Lozano <llozano@chromium.org>
Tested-by: Luis Lozano <llozano@chromium.org>
Reviewed-by: Luis Lozano <llozano@chromium.org>
|
|
This CL creates a pre-push file, to optionally run unittests
when people do 'git push'. The file will need to be copied
into .git/hooks when people set up their toolchain-utils (similar
to the way they need to get the commit-msg file).
BUG=chromium:567918
TEST=Tested in .git/hooks in my toolchain-utils tree.
Change-Id: I706e399554c89fca645b995332d17cb081021088
Reviewed-on: https://chrome-internal-review.googlesource.com/242523
Commit-Ready: Caroline Tice <cmtice@google.com>
Tested-by: Caroline Tice <cmtice@google.com>
Reviewed-by: Yunlian Jiang <yunlian@google.com>
|