Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can I run necessist with nextest? #935

Closed
moodmosaic opened this issue Jan 3, 2024 · 11 comments
Closed

Can I run necessist with nextest? #935

moodmosaic opened this issue Jan 3, 2024 · 11 comments

Comments

@moodmosaic
Copy link
Contributor

What a great tool 👍 💯

When using necessist in Rust, is it agnostic when it comes to the underlying test runner? I'd be interested to run this on a project where tests are run via cargo nextest run.

@smoelius
Copy link
Collaborator

smoelius commented Jan 3, 2024

What a great tool 👍 💯

Thanks. :)

When using necessist in Rust, is it agnostic when it comes to the underlying test runner?

Currently, it is hardcoded to use cargo test:

let mut command = Command::new("cargo");
command.arg("test");

Does your project require cargo nextest?

@moodmosaic
Copy link
Contributor Author

It'd be preferred to use cargo nextest. I can also use cargo test but have to pass some args, for example:

cargo test testnet  -- --test-threads=1

If there's a way to pass arguments to cargo test using necessist then it'd be good enough for me to start.

@smoelius
Copy link
Collaborator

smoelius commented Jan 3, 2024

BAD ADVICE. See below.

I think this should work:

necessist -- -- --test-threads=1

The first -- is to pass arguments to cargo test. The second is to pass arguments to the test executable.

Aside: I'm not opposed to using alternate test runners. But my understanding is that the main benefit to using cargo nextest is to run test executables in parallel. Currently, Necessist runs test executables one-at-a-time, so I can't immediately see a benefit to using cargo nextest. But I might be convinced otherwise.

@moodmosaic
Copy link
Contributor Author

moodmosaic commented Jan 5, 2024

I think this should work:

necessist -- -- --test-threads=1

The first -- is to pass arguments to cargo test. The second is to pass arguments to the test executable.

Thank you. I will try it. Based on what you describe, I think the command should be

# cargo test testnet -- --test-threads=1
necessist -- testnet -- --test-threads=1

Edit: The reason I am asking to verify the command is because I want to filter which tests to run, via

cargo test --package stackslib -- chainstate::stacks::boot::pox_4_tests

And the equivalent in necessist doesn't seem to be picking up the filtering part, resulting in all tests being run:

necessist --reset -- --package stackslib -- chainstate::stacks::boot::pox_4_tests

If this is a bug, I can move it into its own issue.

@smoelius
Copy link
Collaborator

smoelius commented Jan 5, 2024

To filter which tests are run, please try passing a list of test files to Necessist, e.g.:

necessist path-to-file-containing-pox_4_tests ... -- <cargo test args>

This interface is a consequence of wanting to support multiple languages. But I now see that it can be confusing.

I'm not sure what the best "fix" is. Maybe a warning could be issued if the additional arguments match some pattern (e.g., contain --package).

Updated following #935 (comment).

@moodmosaic
Copy link
Contributor Author

Thanks! That worked! Thank you for all the help. This issue can be closed now.


Given all the above, there are errors/warnings when running the tool against a fairly sized codebase (stacks-core), and I am happy to open a separate issue if you think I should.

necessist ./stackslib/src/chainstate/stacks/boot/pox_4_tests.rs \
-- --package stackslib -- chainstate::stacks::boot::pox_4_tests

229 candidates in 1 test file

stackslib/src/chainstate/stacks/boot/pox_4_tests.rs: dry running
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs: mutilating
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:187:5-187:53:
  Warning: Failed to run test `chainstate::stacks::boot::pox_4_tests::pox_extend_transition`
           This may indicate a bug in Necessist. Consider opening an issue at:
           https://github.com/trailofbits/necessist/issues

stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:215:5-216:71:
 Warning: Failed to run test
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:383:9-383:67:
 Warning: Failed to run test
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:416:5-416:63:
 Warning: Failed to run test
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:431:5-431:63:
 Warning: Failed to run test
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:441:9-441:67:
 Warning: Failed to run test
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:445:5-445:48:
 Warning: Failed to run test
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:501:5-504:7:
 Warning: Failed to run test
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:510:9-510:45:
 Warning: Failed to run test
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:627:20-627:32:
 Warning: Failed to run test
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:693:5-693:65:
 Warning: Failed to run test
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:781:5-781:53:
 Warning: Failed to run test
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:797:13-797:47:
 Warning: Failed to run test
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:801:5-804:7:
 Warning: Failed to run test
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:831:5-831:38:
 Warning: Failed to run test
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:840:5-843:7:
 Warning: Failed to run test
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:857:9-857:75:
 Warning: Failed to run test
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:862:9-862:64:
 Warning: Failed to run test
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:869:21-869:52:
 Warning: Failed to run test

████████████████████████████████████████████░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ 124/229
Error: Ctrl-C detected

nikos@ubuntu-linux:~/stacks-core$

@smoelius
Copy link
Collaborator

smoelius commented Jan 6, 2024

Given all the above, there are errors/warnings when running the tool against a fairly sized codebase (stacks-core), and I am happy to open a separate issue if you think I should.

Is the code publicly available so that I might try to reproduce the issue?

If not, do you experience similar behavior with some other code (e.g., in the linked repository)?

@moodmosaic
Copy link
Contributor Author

moodmosaic commented Jan 8, 2024

Is the code publicly available so that I might try to reproduce the issue?

Thank you for asking. Of course, here's how you can reproduce it:

# Install package dependencies (skip if you already have them):
sudo apt install build-essential libclang-dev pkg-config libssl-dev

# Install Rust (skip if you already have it):
curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh
source "$HOME/.cargo/env"

# Clone stacks-core, cd into it, switch to `next` branch:
git clone https://github.com/stacks-network/stacks-core.git && cd stacks-core
git checkout next

# Run `pox_4_tests` via `cargo test`:
cargo test --package stackslib --lib -- chainstate::stacks::boot::pox_4_tests

# Run `pox_4_tests` via `necessist`:
necessist ./stackslib/src/chainstate/stacks/boot/pox_4_tests.rs \
-- --package stackslib -- chainstate::stacks::boot::pox_4_tests

If not, do you experience similar behavior with some other code (e.g., in the linked repository)?

This is the first time I had a chance to use this very interesting tool.

@smoelius
Copy link
Collaborator

smoelius commented Jan 8, 2024

I think this command may do what you want:

necessist stackslib/src/chainstate/stacks/boot/pox_4_tests.rs \
-- --package stackslib chainstate::stacks::boot::pox_4_tests
#                     ^ No `--` here.

I gave you bad advice. The double -- doesn't work. The reason is that Necessist passes -- --exact <testname> at the end of the test command. If the extra arguments contain a --, then the -- before --exact becomes the third in the command, not the second. The end result is an ill-formed test command.

Fortunately, cargo test allows <testname> (e.g., chainstate::stacks::boot::pox_4_tests) to be passed before the --, which is why the above command works.

Aside: RUST_LOG=debug shows the commands that are executed, if you're interested. That's how I debugged this.

@moodmosaic
Copy link
Contributor Author

Thank you! That command executed fine. Now, how do you I interpret the output of necessist after it's finished running? In the output below, each passed occurrence points to a potential bug in the test(?)

nikos@ubuntu-linux:~/stacks-core$ necessist stackslib/src/chainstate/stacks/boot/pox_4_tests.rs -- --package stackslib chainstate::stacks::boot::pox_4_tests
229 candidates in 1 test file
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs: dry running
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs: mutilating
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:215:5-216:71: `peer.config.check_pox_invariants =
        Some((EXPECTED_FIRST_V2_CYCLE, EXPECTED_FIRST_V2_CYCLE + 10));` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:383:9-383:67: `alice_rewards_to_v2_start_checks(latest_block, &mut peer);` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:416:5-416:63: `alice_rewards_to_v2_start_checks(latest_block, &mut peer);` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:431:5-431:63: `alice_rewards_to_v2_start_checks(latest_block, &mut peer);` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:441:9-441:67: `alice_rewards_to_v2_start_checks(latest_block, &mut peer);` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:445:5-445:48: `v2_rewards_checks(latest_block, &mut peer);` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:501:5-504:7: `info!(
        "Block height: {}",
        get_tip(peer.sortdb.as_ref()).block_height
    );` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:510:9-510:45: `info!("----- {cycle_number} -----");` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:627:20-627:32: `.into_iter()` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:693:5-693:65: `check_pox_print_event(stack_tx, common_data, stack_op_data);` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:797:13-797:47: `assert_latest_was_burn(&mut peer);` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:801:5-804:7: `info!(
        "Block height: {}",
        get_tip(peer.sortdb.as_ref()).block_height
    );` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:831:5-831:38: `info!("Submitting stacking txs");` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:840:5-843:7: `info!(
        "Block height: {}",
        get_tip(peer.sortdb.as_ref()).block_height
    );` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:857:9-857:75: `info!("Checking that stackers have STX locked for cycle {cycle}");` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:862:9-862:64: `info!("Checking we have 2 stackers for cycle {cycle}");` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:869:21-869:52: `.filter(|addr| !addr.is_burn())` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:874:17-874:51: `assert_latest_was_burn(&mut peer);` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:887:9-887:51: `info!("Checking we are in prepare phase");` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:890:13-890:47: `assert_latest_was_burn(&mut peer);` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:894:5-894:63: `info!("Checking STX unlocked after {lock_period} cycles");` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:896:9-896:71: `latest_block = peer.tenure_with_txs(&[], &mut coinbase_nonce);` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:897:9-897:43: `assert_latest_was_burn(&mut peer);` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:900:5-900:56: `info!("Checking that stackers have no STX locked");` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:940:13-940:47: `assert_latest_was_burn(&mut peer);` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:944:5-947:7: `info!(
        "Block height: {}",
        get_tip(peer.sortdb.as_ref()).block_height
    );` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:959:9-970:11: `.map(|(key, hash_mode)| {
            let pox_addr = PoxAddress::from_legacy(hash_mode, key_to_stacks_addr(key).bytes);
            txs.push(make_pox_3_lockup(
                key,
                0,
                1024 * POX_THRESHOLD_STEPS_USTX,
                pox_addr.clone(),
                lock_period,
                tip_height,
            ));
            pox_addr
        })` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:961:13-968:16: `txs.push(make_pox_3_lockup(
                key,
                0,
                1024 * POX_THRESHOLD_STEPS_USTX,
                pox_addr.clone(),
                lock_period,
                tip_height,
            ));` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:969:13-969:21: `pox_addr` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:973:5-973:48: `info!("Submitting stacking txs with pox3");` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:976:5-976:56: `info!("Checking that stackers have no STX locked");` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:981:5-981:87: `info!("Checking tx receipts, all `pox3` calls should have returned `(err none)`");` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:1005:5-1008:7: `info!(
        "Block height: {}",
        get_tip(peer.sortdb.as_ref()).block_height
    );` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:1021:9-1021:78: `info!("Checking that stackers have no STX locked for cycle {cycle}");` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:1026:9-1026:57: `info!("Checking no stackers for cycle {cycle}");` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:1028:13-1028:75: `latest_block = peer.tenure_with_txs(&[], &mut coinbase_nonce);` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:1030:13-1030:47: `assert_latest_was_burn(&mut peer);` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:1063:13-1063:47: `assert_latest_was_burn(&mut peer);` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:1067:5-1070:7: `info!(
        "Block height: {}",
        get_tip(peer.sortdb.as_ref()).block_height
    );` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:1096:5-1096:38: `info!("Submitting stacking txs");` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:1116:9-1116:75: `info!("Checking that stackers have STX locked for cycle {cycle}");` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:1121:9-1121:56: `info!("Checking STX locked for cycle {cycle}");` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:1128:21-1128:52: `.filter(|addr| !addr.is_burn())` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:1133:17-1133:51: `assert_latest_was_burn(&mut peer);` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:1146:9-1146:51: `info!("Checking we are in prepare phase");` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:1149:13-1149:47: `assert_latest_was_burn(&mut peer);` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:1156:9-1156:71: `latest_block = peer.tenure_with_txs(&[], &mut coinbase_nonce);` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:1159:5-1162:7: `info!(
        "Block height: {}",
        get_tip(peer.sortdb.as_ref()).block_height
    );` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:1171:9-1171:57: `info!("Checking no stackers for cycle {cycle}");` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:1174:13-1174:47: `assert_latest_was_burn(&mut peer);` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:1177:9-1177:80: `info!("Checking that stackers have no STX locked after cycle {cycle}");` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:1365:5-1365:20: `.expect_tuple()` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:1531:5-1531:20: `.expect_tuple()` passed
██████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████ 229/229

@smoelius
Copy link
Collaborator

smoelius commented Jan 13, 2024

Thank you! That command executed fine.

I'm really glad to hear that.

Now, how do you I interpret the output of necessist after it's finished running? In the output below, each passed occurrence points to a potential bug in the test(?)

All the output tells you is that if you remove those statements/method calls, the tests still pass.

So giving a direct answer to your question ("each passed occurrence points to a potential bug in the test(?)") is difficult.

I suspect that these are false positives and can be ruled out immediately:

stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:501:5-504:7: `info!(
        "Block height: {}",
        get_tip(peer.sortdb.as_ref()).block_height
    );` passed
stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:797:13-797:47: `assert_latest_was_burn(&mut peer);` passed

In other words, it's not surprising for a test to still pass if it prints less information (the former) or performs fewer assertions (the latter).

This one looks more suspicious:

stackslib/src/chainstate/stacks/boot/pox_4_tests.rs:1156:9-1156:71: `latest_block = peer.tenure_with_txs(&[], &mut coinbase_nonce);` passed

It seems to suggest that the latest_block variable need not be updated. That could be a bug, or it could not. One would need to scrutinize the test to tell.

BTW, you might consider creating a necessist.toml file with the following content:

ignored_functions = ["assert_latest_was_burn"]
ignored_macros = ["info"]

Doing so will eliminate many of those likely false positives, and will make Necessist finish sooner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants