mirror of
https://gitlab.isc.org/isc-projects/bind9.git
synced 2025-04-18 09:44:09 +03:00
Update CONTRIBUTING.md and developer doc
Include the recent changes such as: - changes to running system tests - gitlab development workflow - changelog and release note process
This commit is contained in:
Nicki Křížek
parent
d3765a5f35
commit
39485c1f70
@ -11,7 +11,7 @@ See the COPYRIGHT file distributed with this work for additional
|
||||
information regarding copyright ownership.
|
||||
-->
|
||||
## BIND 9 Source Access and Contributor Guidelines
|
||||
*May 28, 2020*
|
||||
*Nov 26, 2024*
|
||||
|
||||
### Contents
|
||||
|
||||
@ -72,13 +72,13 @@ To clone the repository, use:
|
||||
> $ git clone https://gitlab.isc.org/isc-projects/bind9.git
|
||||
|
||||
Release branch names are of the form `bind-9.X`, where X represents the second
|
||||
number in the BIND 9 version number. So, to check out the BIND 9.18
|
||||
number in the BIND 9 version number. So, to check out the BIND 9.20
|
||||
branch, use:
|
||||
|
||||
> $ git checkout bind-9.18
|
||||
> $ git checkout bind-9.20
|
||||
|
||||
Whenever a branch is ready for publication, a tag is placed of the
|
||||
form `v9.X.Y`. The 9.18.0 release, for instance, is tagged as `v9.18.0`.
|
||||
form `v9.X.Y`. The 9.20.0 release, for instance, is tagged as `v9.20.0`.
|
||||
|
||||
The branch in which the next major release is being developed is called
|
||||
`main`.
|
||||
@ -121,8 +121,9 @@ patch will be applied.
|
||||
#### <a name="bind"></a>BIND code
|
||||
|
||||
Patches for BIND may be submitted directly via merge requests in
|
||||
[ISC's GitLab](https://gitlab.isc.org/isc-projects/bind9/) source
|
||||
repository for BIND.
|
||||
[ISC's GitLab](https://gitlab.isc.org/isc-projects/bind9/) source repository for
|
||||
BIND. Please contact ISC and provide your GitLab username in order to be allowed
|
||||
to fork the project and submit merge requests.
|
||||
|
||||
Patches can also be submitted as diffs against a specific version of
|
||||
BIND -- preferably the current top of the `main` branch. Diffs may
|
||||
@ -144,8 +145,8 @@ we're busy with other work, it may take us a long time to get to it.
|
||||
To ensure your patch is acted on as promptly as possible, please:
|
||||
|
||||
* Try to adhere to the [BIND 9 coding style](doc/dev/style.md).
|
||||
* Run `make check` to ensure your change hasn't caused any
|
||||
functional regressions.
|
||||
* Run unit and system tests to ensure your change hasn't caused any
|
||||
functional regressions (these can be checked in the CI pipeline).
|
||||
* Document your work, both in the patch itself and in the
|
||||
accompanying email.
|
||||
* In patches that make non-trivial functional changes, include system
|
||||
@ -156,12 +157,12 @@ To ensure your patch is acted on as promptly as possible, please:
|
||||
##### Changes to `configure`
|
||||
|
||||
If you need to make changes to `configure`, you should not edit it
|
||||
directly; instead, edit `configure.in`, then run `autoconf`. Similarly,
|
||||
instead of editing `config.h.in` directly, edit `configure.in` and run
|
||||
directly; instead, edit `configure.ac`, then run `autoconf`. Similarly,
|
||||
instead of editing `config.h.in` directly, edit `configure.ac` and run
|
||||
`autoheader`.
|
||||
|
||||
When submitting a patch as a diff, it's fine to omit the `configure`
|
||||
diffs to save space. Just send the `configure.in` diffs and we'll
|
||||
diffs to save space. Just send the `configure.ac` diffs and we'll
|
||||
generate the new `configure` during the review process.
|
||||
|
||||
##### Documentation
|
||||
|
||||
@ -43,8 +43,8 @@ The code review process is a dialog between the original author and the
|
||||
reviewer. Code inspection, including documentation and tests, is part of
|
||||
this. Compiling and running the resulting code should be done in most
|
||||
cases, even for trivial changes, to ensure that it works as intended. In
|
||||
particular, a full regression test (`make` `check`) must be run for every
|
||||
modification so that unexpected side-effects are identified.
|
||||
particular, all checks in the CI pipeline must pass run for every modification
|
||||
so that unexpected side-effects are identified.
|
||||
|
||||
When a problem or concern is found by the reviewer, these comments are
|
||||
placed on the merge request in GitLab so the author can respond.
|
||||
@ -78,18 +78,25 @@ Documentation is also reviewed. This includes all user-facing text,
|
||||
including log messages, manual pages, user manuals and sometimes even
|
||||
comments; they must be clearly written and consistent with existing style.
|
||||
|
||||
#### GitLab development workflow
|
||||
|
||||
Every change is ultimately submitted as a GitLab merge request (MR) and reviewed
|
||||
there. The specifics of the workflow are documented in [BIND development
|
||||
workflow](https://gitlab.isc.org/isc-projects/bind9/-/wikis/BIND-development-workflow).
|
||||
Take note of the section about MR title and description, which are used to
|
||||
generate changelog entries and release notes. These are also subject to the
|
||||
review process.
|
||||
|
||||
#### Steps in code review:
|
||||
|
||||
* Read the diff
|
||||
* Read accompanying notes in the ticket
|
||||
* Apply the diff to the appropriate branch
|
||||
* Run `configure` (using at least `--enable-developer`)
|
||||
* Build
|
||||
* Read the documentation, if any
|
||||
* Read the tests
|
||||
* Run the tests
|
||||
* Ensure the CI passes
|
||||
<br>(In some cases it may be appropriate to run tests against code
|
||||
from before the change to ensure that they fail as expected.)
|
||||
* Review the MR description and title (refer to GitLab development workflow)
|
||||
|
||||
#### Things we look for
|
||||
|
||||
@ -128,73 +135,16 @@ tests and documentation will reduce delay.
|
||||
|
||||
### <a name="testing"></a> Testing
|
||||
|
||||
#### <a name="systest"></a> Running system tests
|
||||
When you submit a merge request, it triggers a CI pipeline which executes unit
|
||||
and system tests on various platforms. You should pay attention to any failures,
|
||||
as some can only occur in specific environments. Getting the CI to pass is a
|
||||
good start when preparing the merge request for the review.
|
||||
|
||||
To enable system tests to work, we first need to create the test loopback
|
||||
interfaces (as root):
|
||||
#### <a name="systest"></a> System tests
|
||||
|
||||
$ cd bin/tests/system
|
||||
$ sudo sh ifconfig.sh up
|
||||
$ cd ../../..
|
||||
|
||||
To run the tests, build BIND (be sure to use --with-cmocka to run unit
|
||||
tests), then run `make` `check`. An easy way to check the results:
|
||||
|
||||
$ make check 2>&1 | tee /tmp/check.out
|
||||
$ grep -A 10 'Testsuite summary' /tmp/check.out
|
||||
|
||||
This will show all of the test results. One or two "R:SKIPPED" is okay; if
|
||||
there are a lot of them, then you probably forgot to create the loopback
|
||||
interfaces in the previous step. (NOTE: the summary of tests that appears at
|
||||
the end of `make` `check` only summarizes the system test results, not the
|
||||
unit tests, so you can't rely on it to catch everything.)
|
||||
|
||||
To run only the system tests, omitting unit tests:
|
||||
|
||||
$ make test
|
||||
|
||||
To run an individual system test:
|
||||
|
||||
$ make -C bin/tests/system/ check TESTS=<testname> V=1
|
||||
|
||||
Or:
|
||||
|
||||
$ TESTS= make -e all check
|
||||
$ cd bin/tests/system
|
||||
$ sh run.sh <testname>
|
||||
|
||||
System tests are in separate directories under `bin/tests/system`.
|
||||
For example, the "dnssec" test is in `bin/tests/system/dnssec`.
|
||||
|
||||
#### Writing system tests
|
||||
|
||||
The following standard files are found in system test directories:
|
||||
|
||||
- `prereq.sh`: run at the beginning to determine whether the test can be run at all; if not, we see R:SKIPPED
|
||||
|
||||
- `setup.sh`: sets up the preconditions for the tests
|
||||
|
||||
- `tests.sh`: runs all the test cases. A non-zero return value results in R:FAIL
|
||||
|
||||
- `ns[X]`: these subdirectories contain test name servers that can be
|
||||
queried or can interact with each other. (For example, `ns1` might be
|
||||
running as a root server, `ns2` as a TLD server, and `ns3` as a recursive
|
||||
resolver.) The value of X indicates the address the server listens on:
|
||||
for example, `ns2` listens on 10.53.0.2, and ns4 on 10.53.0.4. All test
|
||||
servers use port 5300 so they don't need to run as root. All servers
|
||||
log at the highest debug level, and the logs are captured in the file
|
||||
`nsX/named.run`.
|
||||
|
||||
- `ans[X]`: like `ns[X]`, but these are simple mock name servers
|
||||
implemented in perl; they are generally programmed to misbehave in ways
|
||||
`named` wouldn't, so as to exercise `named`'s ability to interoperate with
|
||||
badly behaved name servers. Logs, if any, are captured in `ansX/ans.run`.
|
||||
|
||||
All test scripts source the file `bin/tests/system/conf.sh` (which is
|
||||
generated by `configure` from `conf.sh.in`). This script provides
|
||||
functions and variables pointing to the binaries under test; for example,
|
||||
`DIG` contains the path to `dig` in the build tree being tested, `RNDC`
|
||||
points to `rndc`, `SIGNZONE` to `dnssec-signzone`, etc.
|
||||
If you want to run the system tests locally, please refer to [BIND9 System Test
|
||||
Framework](bin/tests/system/README.md) for information about running and writing
|
||||
system tests.
|
||||
|
||||
#### <a name="unittest"></a> Building unit tests
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user