While many testcases don't meet flake8 requirements right now
(the most prevalent one is the "*" import, but we also have
expected output strings with lines longer than 88 characters),
we can still improve the testcases using black to format them.
If anything, this will avoid to break many of flake8's rules.
This commit changes the .pre-commit-config.yaml file to remove
the exclusion for testsuite/tests/*. From there, all the files
in testsuite/tests/ were reformatted via:
$ pre-commit run --all
Change-Id: Ia4957899cabf909f401bb2a242ed7035d89f1df9
This commit introduces a new run attribute in our testsuite's
TestCase class as a way to allow testcases to call this method
instead of instantiating the Run class. All testcases are then
modified to use this method, by running the following perl
command:
$ cd testsuite/tests
$ perl -pi -e 's/\bRun\(/testcase.run(/g' **/run_test.py
The testcases are then modified to remove the unused import using
the following perl command:
$ perl -pi -e 's/\s+Run,//g' **/run_test.py
The goal is to help the transition to pytest, where the testcases
will be using the "run" method to call external commands such as
Git. The difference is that the pytest version of this method
will do more than being a simple wrapper (e.g. it will take care
of the default working directory).
TN: U530-006
Change-Id: I1823ef4331d8d16da6cf5f4a4efdbb667c68848d
This is another preparation patch towards the transition of
the testsuite framework to pytest (as part of the transition
to Python 3.x), where we will use a fixture called "testcase"
to replace the TestCase object (self).
To facilitate the transition as well as the review of that (future)
transition, this commit renames the "self" parameter into "testcase".
Note that the change was performed entirely automatically thanks to
the following perl command:
perl -pi -e 's/\bself\b/testcase/g' **/run_test.py
It's a departure from the convention that the object for which
a method is called be called "self", but it's only a temporary
situation; once we transition the testsuite to pytest, we will
no longer have classes -- just simple functions.
TN: U530-006
Change-Id: Id9cf32c8a8f1ed0c2fd4236f303c8fbd7b4894c1
This commit removes all the calls to assertTrue that were still
left. In this commit, all the changes were made by hand.
This is preparation work for the transition of the testsuite out
of gnatpython (Python 2.x only), and relying on pytest instead,
which will allow us to run the testsuite using Python 3.x.
TN: U530-006
Change-Id: I0f1925341954ff60a01149dd2075f87dfa3e573e
TestCase.assertTrue does not bring anything compared to just using
the assert statement. So this commit uses the following perl command
to automatically rewrite all calls which fit in a single line
(which is most of our calls, but not all of them):
$ cd testsuite/tests
$ perl -pi -e 's/\S*\.assertTrue\((.*)\)\s*$/assert \1/' \
**/run_test.py
This is preparation work for the transition of the testsuite out
of gnatpython (Python 2.x only), and relying on pytest instead,
which will allow us to run the testsuite using Python 3.x.
There are still a handful of calls to assertTrue due to them
being spread over more than one line. Those will be treated in
a separate commit to help with review, as those will be handled
manually.
TN: U530-006
Change-Id: I58daa2d6027e82edf5dc664159d08d9d9eaf2d7b
This commit removes a bit of code aimed at supporting the output
from an old version of Git, and does so by modifying the .out
attribute of the Run object we got from running Git. Unfortunately,
this won't work when we switch to Python 3.x and E3's Run class,
as the "out" attribute has become a property instead, so we cannot
modify its value. Since the version of Git we're trying to support
is very old at this point (latest release on 1.7 branch was Oct 2012),
we don't really need to keep that code anymore.
Found while working on U530-006.
Change-Id: I0ad83b00bf1d9e46ab67fd9bc50400f75308f32b
A number of cvs_check.py scripts provided by some testcase use a mix
of spaces and tabs for indentation, in a way that causes some errors
when evaluating those scripts using Python 3.x. This commit enhances
those scripts to only use spaces for indentation.
Found while working on U530-006.
Change-Id: I17076204b470dc7cfc4d5ef81b40ef7f2217762b
This commit makes sure that the stdout and stderr output has been
flushed so that no output is left buffered by the time we call os.fork.
Otherwise, if there was any output still buffered, we would be getting
the same output printed multiple times, once by the parent process,
and once by each child process.
Found while working on the transition to Python 3.x (U530-006).
TN: U619-004
Change-Id: I9c38bfc410041e58de2a8f5d7d340f7bff599df4
I noticed while looking at this testcase that the name of the test_xxx
method used to perform the testing in this testcase did not match what
the testing was trying to do (likely because this testcase was created
by first copying an existing testcase and then modifying it). This commit
fixes the issue by renaming that method.
Found while working on: U530-006.
Change-Id: Icbddfd9bc2ea801df28b68219dcc09bfe5e9e62b
I noticed while working on the transition to Python 3.x that
this argument is not actually used, so this commit removes it.
It makes the API for this function simpler and also helps avoid
some pitfalls (as shown by one of the limitations we documented
that this commit removes).
TN: U530-006
Change-Id: I5ced7c6a979c5381d35498b06f1af8c569a3f9ed
As part of the transition to Python 3.x, we will start adding some code
which should only be executed when running the hooks using Python 3.x.
In the meantime, until the testsuite gets transitionned to Python 3.x,
we still want the coverage report for Python 2.x to remain clean, so
this commit adds a new exclude line to the default one: Any line with
"pragma: py3-only" should now be excluded from the coverage report.
As for the future, what we will do when the transition to Python 3.x
actually happens, is change the config again to exclude lines marked
"py2-only" (or remove the Python-2.x-only code directly, if that makes
better sense).
TN: U530-006
Change-Id: I67a1fe27dbfe614c1dba5176d78f1d8d1fe9cd35
The coverage.sh script creates a coverage.rc file, but this file wasn't
actually used during coverage generation. This change makes sure that
it is.
For now, this shouldn't make any difference, but this will help us
later for providing additional options during report generation
(in particular, it will allow us to add coverage exclusion patterns,
which will become handy to exclude code which is only meant to be
executed with a given version of Python).
TN: U530-006
Change-Id: Ifbba6dfb9a9347f7d5680b6cc464378282653813
This is a preparation patch for the transition to Python 3.x.
Currently, we have the following code:
| files_to_check = filter(needs_style_check_p, files_to_check)
| if not files_to_check:
| debug("style_check_commit: no files to style-check")
| return
With Python 2.x, the "filter" function constructs a list, so
we can indeed check for emptiness using the logical operator "not".
With Python 3.x, however, filter returns an iterator, and using
the logical operator "not" no longer has the same semantics at all.
The easiest way to handle this is to simply convert the result to
a tuple.
TN: U530-006
Change-Id: I334f2657728405226d7da577e893b7c64a3220fb
This commit updates the pre-commit hook to black version 21.5b1.
The hooks where then re-run on all files to update their formatting
to this new version of black.
Change-Id: Ib0866745ef8432cf93380a4d83fa23a479eb4a49
The "filename_list" of the style_check_files function is expected
to be a list. However, as it happens, we have a call to this function
where the filename_list parameter was computed using the "filter"
function.
files_to_check = filter(needs_style_check_p, files_to_check)
style_check_files(files_to_check, new_rev, project_name)
This is no problem when trying to iterate on the list alone, but
at one point inside the "style_check_files" funtion, we try to
add to it like so:
for filename in filename_list + aux_files:
With Python 3.x, this causes a crash because the "filter" function
now returns a generator rather than a list. Seems it seems desirable
to allow "style_check_files" to accept any iterable as "filename_list",
this commit modifies the code to allow it.
TN: U530-006
Change-Id: I8e5caaf09d177096788c522d7bff8168eb158ec7
This commit changes some code which currently only works with
Python 2.x, to make it compatible with both Python 2.x and
Python 3.x.
TN: U530-006
Change-Id: I4874452cdbb26410dfe7a2d491ed400e6b038029
This commit modifies a conditional expression to avoid comparing
the output from a git command with the empty string. With Python 3.x,
this wouldn't work unless we decoded the output first or compared
against the empty *byte* string. But since all we're interested in
is knowing whether the output is empty or not, we can use a simple
"truth" test instead, thus avoiding having to worry about the type
of the output entirely.
TN: U530-006
Change-Id: I0067cb6bfb37da4e1782a045beac19d970d7718f
This commit is inspired by the fact that I couldn't understand
why I was skipping the first character from the output of a Git
command whe computing a commit's "diff", like so):
diff = git.show(commit.rev, [...], pretty="format:|")[1:]
(emphasis on the "[1:]" at the end).
To understand, I remove the subscripting and reran the testsuite
without it to see what failures I would get. This gave me
the answer, which is we were intentionally starting the "format:"
string with a "|", and so we needed to strip that extra character.
That's when I found a comment I wrote; I didn't see it at first
because it was placed further up:
# For the diff, there is one subtlelty:
# Git commands calls strip on the output, which is usually
# a good thing, but not in the case of the diff output.
# Prevent this from happening by putting an artificial
# character at the start of the format string, and then
# by stripping it from the output.
This may have made sense back when I wrote that comment, but
we no longer strip the start of the output anymore (see commit
af06d5ea54). So I decided to
simplify the code by removing the extraneous character in
the "format:" string.
As it happens, this revealed that git behaves slightly differently
when given an empty "format:" string. Before:
| $ git show -p -M --stat --pretty="format:|" HEAD
| |---
| hooks/git.py | 4 +---
| 1 file changed, 1 insertion(+), 3 deletions(-)
|
| diff --git a/hooks/git.py b/hooks/git.py
| index fe2b36b..0669111 100644
| [snip]
After (removing the "|" in the "format:" string):
| $ git show -p -M --stat --pretty="format:" HEAD
| hooks/git.py | 4 +---
| 1 file changed, 1 insertion(+), 3 deletions(-)
|
| diff --git a/hooks/git.py b/hooks/git.py
| [snip]
What we can see is that the "---" separate line is no longer shown
in the second command.
Rather than forcing Git to print it, or rather than staying with
the existing code, this commit simply hardcodes the separator line.
One minor bonus of doing it this way is that, if Git decides to
change that separator, this won't affect us, and thus we won't
have to change hundreds of tests accordingly.
And by doing so, this revealed that there was actually an inconsistency
in the formatting produced by Git: In some cases (e.g. merge commits),
it became apparent that Git was omitting this "---" separator line,
even when the "format:" string was empty. The corresponding testcases
where the inconsistency showed up were adjusted to match the new
behavior, which is consisdered (slightly) better, because more
consistent.
Found while working on U530-006 (transition to Python 3.x).
Change-Id: Ifc473fa471ba618e11c3c4bcc6d83cc6f82fc6bf
The use of the "git log" command allows us to avoid having to
post-process the command's output in order to extra the subject.
Found while working on U530-006 (transition to Python 3).
Change-Id: I96e9545deece7a2094f9637b75d5e7fb6f334d9a
