Skip to content

Commit

Permalink
Merge bitcoin#29228: test: Remove all-lint.py script
Browse files Browse the repository at this point in the history
fa2b95c test: Remove all-lint.py script (MarcoFalke)
fadb06c doc: move-only lint docs to one place (MarcoFalke)

Pull request description:

  Seems confusing to have a test runner that calls another runner (`all-lint.py`), which calls a subset of the lint tests.

  Fix that by just calling this subset of lint tests in the test runner directly, and remove the now unused `all-lint.py`.

  To run all lint checks locally, refer to the documentation: https://github.com/bitcoin/bitcoin/blob/master/test/lint/README.md#running-locally

ACKs for top commit:
  kevkevinpal:
    ACK [fa2b95c](bitcoin@fa2b95c)
  achow101:
    ACK fa2b95c
  TheCharlatan:
    ACK fa2b95c
  pablomartin4btc:
    tACK fa2b95c
  brunoerg:
    utACK fa2b95c

Tree-SHA512: 43fac9acb4e9a6744d695dd49c7202e19ab4bf480f4cccff768647d0157a065f40e6ad70b9f6a65ba42048cc5fa9834365aa8e7aa0ed64c09e0cd4eb8dccb831
  • Loading branch information
achow101 committed Jan 18, 2024
2 parents 03c5b00 + fa2b95c commit ac3901e
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 66 deletions.
30 changes: 1 addition & 29 deletions test/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -326,35 +326,7 @@ Use the `-v` option for verbose output.

### Lint tests

#### Dependencies

| Lint test | Dependency |
|-----------|:----------:|
| [`lint-python.py`](lint/lint-python.py) | [flake8](https://gitlab.com/pycqa/flake8)
| [`lint-python.py`](lint/lint-python.py) | [lief](https://github.com/lief-project/LIEF)
| [`lint-python.py`](lint/lint-python.py) | [mypy](https://github.com/python/mypy)
| [`lint-python.py`](lint/lint-python.py) | [pyzmq](https://github.com/zeromq/pyzmq)
| [`lint-python-dead-code.py`](lint/lint-python-dead-code.py) | [vulture](https://github.com/jendrikseipp/vulture)
| [`lint-shell.py`](lint/lint-shell.py) | [ShellCheck](https://github.com/koalaman/shellcheck)
| [`lint-spelling.py`](lint/lint-spelling.py) | [codespell](https://github.com/codespell-project/codespell)

In use versions and install instructions are available in the [CI setup](../ci/lint/04_install.sh).

Please be aware that on Linux distributions all dependencies are usually available as packages, but could be outdated.

#### Running the tests

Individual tests can be run by directly calling the test script, e.g.:

```
test/lint/lint-files.py
```

You can run all the shell-based lint tests by running:

```
test/lint/all-lint.py
```
See the README in [test/lint](/test/lint).

# Writing functional tests

Expand Down
30 changes: 25 additions & 5 deletions test/lint/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,36 @@ result is cached and it prevents issues when the image changes.
test runner
===========

To run the checks in the test runner outside the docker, use:
To run all the lint checks in the test runner outside the docker, use:

```sh
( cd ./test/lint/test_runner/ && cargo fmt && cargo clippy && cargo run )
```

#### Dependencies

| Lint test | Dependency |
|-----------|:----------:|
| [`lint-python.py`](lint/lint-python.py) | [flake8](https://gitlab.com/pycqa/flake8)
| [`lint-python.py`](lint/lint-python.py) | [lief](https://github.com/lief-project/LIEF)
| [`lint-python.py`](lint/lint-python.py) | [mypy](https://github.com/python/mypy)
| [`lint-python.py`](lint/lint-python.py) | [pyzmq](https://github.com/zeromq/pyzmq)
| [`lint-python-dead-code.py`](lint/lint-python-dead-code.py) | [vulture](https://github.com/jendrikseipp/vulture)
| [`lint-shell.py`](lint/lint-shell.py) | [ShellCheck](https://github.com/koalaman/shellcheck)
| [`lint-spelling.py`](lint/lint-spelling.py) | [codespell](https://github.com/codespell-project/codespell)

In use versions and install instructions are available in the [CI setup](../ci/lint/04_install.sh).

Please be aware that on Linux distributions all dependencies are usually available as packages, but could be outdated.

#### Running the tests

Individual tests can be run by directly calling the test script, e.g.:

```
test/lint/lint-files.py
```

check-doc.py
============
Check for missing documentation of command line options.
Expand Down Expand Up @@ -59,7 +83,3 @@ To do so, add the upstream repository as remote:
```
git remote add --fetch secp256k1 https://github.com/bitcoin-core/secp256k1.git
```

all-lint.py
===========
Calls other scripts with the `lint-` prefix.
23 changes: 0 additions & 23 deletions test/lint/all-lint.py

This file was deleted.

32 changes: 23 additions & 9 deletions test/lint/test_runner/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// file COPYING or https://opensource.org/license/mit/.

use std::env;
use std::fs;
use std::path::PathBuf;
use std::process::Command;
use std::process::ExitCode;
Expand All @@ -29,8 +30,8 @@ fn check_output(cmd: &mut std::process::Command) -> Result<String, LintError> {
}

/// Return the git root as utf8, or panic
fn get_git_root() -> String {
check_output(git().args(["rev-parse", "--show-toplevel"])).unwrap()
fn get_git_root() -> PathBuf {
PathBuf::from(check_output(git().args(["rev-parse", "--show-toplevel"])).unwrap())
}

fn lint_subtree() -> LintResult {
Expand Down Expand Up @@ -94,11 +95,24 @@ fn lint_doc() -> LintResult {
}

fn lint_all() -> LintResult {
if Command::new("test/lint/all-lint.py")
.status()
.expect("command error")
.success()
{
let mut good = true;
let lint_dir = get_git_root().join("test/lint");
for entry in fs::read_dir(lint_dir).unwrap() {
let entry = entry.unwrap();
let entry_fn = entry.file_name().into_string().unwrap();
if entry_fn.starts_with("lint-")
&& entry_fn.ends_with(".py")
&& !Command::new("python3")
.arg(entry.path())
.status()
.expect("command error")
.success()
{
good = false;
println!("^---- failure generated from {}", entry_fn);
}
}
if good {
Ok(())
} else {
Err("".to_string())
Expand All @@ -110,10 +124,10 @@ fn main() -> ExitCode {
("subtree check", lint_subtree),
("std::filesystem check", lint_std_filesystem),
("-help=1 documentation check", lint_doc),
("all-lint.py script", lint_all),
("lint-*.py scripts", lint_all),
];

let git_root = PathBuf::from(get_git_root());
let git_root = get_git_root();

let mut test_failed = false;
for (lint_name, lint_fn) in test_list {
Expand Down

0 comments on commit ac3901e

Please sign in to comment.