1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
|
* Standard exception rules apply: if there's a reason to break the rule that
applies to a specific case but not generally, break the rule. If it applies
generally, change the policy first.
* Follow PEP8.
* Single quotes for strings, with one exception: double quotes for English
prose or phrases, such as string that might be translatable, to avoid
collisions with apostrophes (when writing as well as for the future).
* If a line is too long, break it immediately after a (, and indent the next
line by one extra indent only (not hanging).
* If a method call goes to multiple lines, use one parameter per line, each
ending in a trailing comma including the last parameter, and the closing
bracket on a separate final line.
* If more than a few parameters, use the keyword argument form in method calls
so that the reader doesn't have to depend on argument ordering to understand
the code.
* Avoid wrapping for, if, while and similar statements across multiple lines;
make them shorter instead (eg. by calculating things and storing the results
in variables first, and then using those).
* Where appropriate, consider making constants scoped within a method or
function a keyword parameter with a default instead.
* Subcommand modules must be leaves. No cycles in the import graphs.
* In snapcraft.yaml, cache external dependencies on our infrastructure
wherever possible.
* Divide imports into three sections separated by a blank line:
1) Standard library imports (eg. collections, time, unittest.mock).
2) Third party library imports (eg. pytest).
3) Project-specific imports (eg. `import gitubuntu.git_repository as
git_repository`).
This avoids hiding a small number of project-specific or third party
library imports into a wall of standard library imports, giving the
reader an immediately clearer view of the less usual entries being
brought into the namespace.
* Keep imports sorted. This keeps diffs cleaner.
* Keep the test suite always passing and tests marked "xfail" always
failing. We use CI and `xfail_strict=true` in `pytest.ini` to enforce
this.
If a bugfix comes with a test and the test can be arranged to work
without its fix, then a good technique is to add the test first in an
earlier commit, decorated with `@pytest.mark.xfail` or the
`pytest.param`/`marks` equivalent.
Commits that cause previously fixed tests to pass (such as bugfix
commits with a test added previously as above) should drop the xfail
at the same time.
In other words, where possible we use `xfail` to keep the test suite
informed about known problems in the source tree at commit level
granularity.
This helps the reviewer easily check the correctness of any tests
being added, as well as their fixes. It also helps to keep CI "always
green", making future bisection and other code archaeology easier.
Testing Strategy
----------------
Good test coverage (as reported by `py.test-3 --cov=gitubuntu
--cov-report=term-missing gitubuntu` for example) is good to catch
errors in less common code paths. Unit tests can help achieve this.
However, in this project this usually isn't sufficient.
This project is mostly "glue", connecting git with Launchpad, with
Debian and Ubuntu source packages, and with various existing CLI
tooling. There is a risk of "getting the glue wrong" because we've
misunderstood an external API or the behaviour of these external
components.
When this happens, unit tests would pass, because we'd be calling the
external components exactly how we think we should, and correctly
handling data received back as we fake the form that we think we'd get
it. But we would be missing testing that we're actually calling these
components correctly, and that we actually get the data back that we
expect.
Experience has shown that unit tests are therefore not enough.
Integration tests are important to catch these problems, and do so
regularly.
It isn't practical to speak to Launchpad live in a test, but we can
speak to a local git instance, and to local packaging tooling. So, in
general, we should do that. Then we will be testing that we are speaking
to git and to tooling CLIs correctly.
We use [pytest fixtures](https://docs.pytest.org/en/latest/fixture.html)
to help us with this. Use of pytest's built-in `tmpdir` fixture is very
common, as is `repo` and `pygit2_repo` as defined in
`gitubuntu.test_fixtures`.
|
