feat: add cargo nextest support #144

skyzh · 2022-03-17T14:49:10Z

Signed-off-by: Alex Chi iskyzh@gmail.com

cargo-nextest is quite popular recently, but it lacks coverage support. In this PR, I added a new nextest command to run tests.

I've tested it locally and it works. On my own project, it shows (nearly) the same coverage as normal cargo llvm-cov.

cargo llvm-cov nextest

Not sure if it is good to simply add a nextest command... Maybe it would be better to allow users to run arbitrary command in cargo llvm-cov <user command>?

Thanks for reviewing!

taiki-e

Thanks for the PR!

Not sure if it is good to simply add a nextest command... Maybe it would be better to allow users to run arbitrary command in cargo llvm-cov <user command>?

Ideally, I would like cargo-llvm-cov to work with other subcommands as well.
However, I don't know what a good way to "passthrough arguments to an external subcommand" using clap, so I'm planning to move CLI parsing to lexopt before doing that. (Like I've done in other crates in the past 1, 2, 3)

taiki-e · 2022-03-17T17:00:37Z

src/cli.rs

+        max_term_width(MAX_TERM_WIDTH),
+        setting(AppSettings::DeriveDisplayOrder)
+    )]
+    Nextest,


Options after the nextest subcommand do not seem to be recognized.

$ cargo llvm-cov nextest --all-features error: Found argument '--all-features' which wasn't expected, or isn't valid in this context If you tried to supply `--all-features` as a value rather than a flag, use `-- --all-features` USAGE: cargo llvm-cov nextest For more information try --help

I believe https://github.com/facebookincubator/cargo-guppy/blob/main/tools/cargo-hakari/src/command.rs#L250 should work.

That's awesome! I'll have a try :)

Signed-off-by: Alex Chi <iskyzh@gmail.com>

skyzh · 2022-03-18T00:37:25Z

I've updated the code to parse the extra args. PTAL, thanks! cc @taiki-e

Signed-off-by: Alex Chi <iskyzh@gmail.com>

taiki-e

Looks great!

bors r+

bors · 2022-03-18T13:29:06Z

Build succeeded:

ci

145: Add simple test for `cargo llvm-cov nextest` r=taiki-e a=taiki-e follow-up #144 Co-authored-by: Taiki Endo <te316e89@gmail.com>

taiki-e · 2022-03-18T15:52:50Z

Published in 0.2.4.

Also, I've added nextest support to the installer of cargo-llvm-cov for GitHub Actions and updated installation docs.

cargo-llvm-cov/README.md

Lines 419 to 424 in d070f37

    
           When used with [nextest]: 
        
           ```yml 
        
           - uses: taiki-e/install-action@cargo-llvm-cov 
        
           - uses: taiki-e/install-action@nextest 
        
           ```

sunshowers · 2022-03-18T19:27:26Z

@skyzh @taiki-e I was playing around with this and I noticed that the --html option didn't appear to work and generated the text output instead. Wondering if the CLI parsing can be fixed to enable this.

taiki-e · 2022-03-18T19:50:31Z

Ah, flags before nextest subcommand seem to be ignored.

Flags after nextest subcommand (works):

$ cargo llvm-cov nextest --html
...
Summary [   1.798s] 861 tests run: 861 passed, 0 skipped

Finished report saved to /Users/taiki/projects/portable-atomic/target/llvm-cov/html

Flags before nextest subcommand (not work):

$ cargo llvm-cov --html nextest
Summary [   1.853s] 861 tests run: 861 passed, 0 skipped
Filename                              Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
...
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                     528               126    76.14%         165                13    92.12%        1469               132    91.01%           0                 0         -

sunshowers · 2022-03-18T20:20:52Z

Aha, thanks! It's a little confusing because of how the passthrough options work I guess.

sunshowers · 2022-03-18T20:25:09Z

Looking at this more, I think the support for passthrough args is slightly broken. For example, nextest has a --cargo-profile option to set the Cargo profile.

% cargo llvm-cov nextest --cargo-profile release   
error: error: Found argument '--cargo-profile' which wasn't expected, or isn't valid in this context

        If you tried to supply `--cargo-profile` as a value rather than a flag, use `-- --cargo-profile`

USAGE:
    cargo llvm-cov [OPTIONS] [-- <ARGS>...] [SUBCOMMAND]

For more information try --help

% cargo llvm-cov nextest -- --cargo-profile release
error: error: Found argument '--cargo-profile' which wasn't expected, or isn't valid in this context

        If you tried to supply `--cargo-profile` as a value rather than a flag, use `-- --cargo-profile`

USAGE:
    cargo llvm-cov [OPTIONS] [-- <ARGS>...] [SUBCOMMAND]

For more information try --help

% cargo llvm-cov nextest -- -- --cargo-profile release
   Compiling quick-junit v0.1.5 (/home/rain/dev/nextest/quick-junit)
   Compiling nextest-metadata v0.2.1 (/home/rain/dev/nextest/nextest-metadata)
   (this works)

sunshowers · 2022-03-18T20:28:22Z

Personally I suspect it would be easiest if the arguments to nextest were handled after --, and also that the Cargo-specific arguments (like -p) weren't supported for the nextest subcommand. So you'd have to run:

cargo llvm-cov nextest --html -- -p my-package

which makes the separation of arguments pretty clear.

What do you think?

skyzh · 2022-03-19T04:18:27Z

Some args need to be fed to both nextest and llvm-cov to get things work correctly. For example, if user specifies --exclude in cli, it seems that both nextest and llvm-cov needs to know this in order to correctly generate the report.

cargo-llvm-cov/src/cli.rs

Lines 137 to 139 in d070f37

    
               /// Exclude packages from both the test and report 
        
               #[clap(long, multiple_occurrences = true, value_name = "SPEC", requires = "workspace")] 
        
               pub(crate) exclude: Vec<String>,

Maybe we'll need to have a stable cli interface for nextest, and make the full opt parsing of nextest part of the llvm-cov?

sunshowers · 2022-03-19T04:59:34Z

Ahhh I see. Well, nextest's interface is stable (i.e append-only, though new features are being added all the time).

Specifically around test names, there's work going on in nextest-rs/nextest#117 to select specific tests in specific packages. There's also experimental support for reusing builds across machines by archiving the target directory, for example: https://nexte.st/book/reusing-builds.html

I'm wondering if nextest can provide the information llvm-cov needs as a JSON blob. What information do you need?

taiki-e · 2022-03-19T06:51:03Z

Based on my experience with cargo-hack and cargo-minimal-versions (and some unpublished cargo subcommands), I think the minimal information that is needed for partial parsing arguments in such a subcommand is basically only information about the short flags, which have no value.

I believe the same approach could be used for cargo-llvm-cov.

197: Switch from clap to lexopt r=taiki-e a=taiki-e Fixes #151, #144 (comment), and #144 (comment) Co-authored-by: Taiki Endo <te316e89@gmail.com>

taiki-e · 2022-09-10T08:33:38Z

Arguments-related bugs have been fixed in 0.5.0.

skyzh mentioned this pull request Mar 17, 2022

Compatibility with coverage tools? nextest-rs/nextest#56

Closed

taiki-e reviewed Mar 17, 2022

View reviewed changes

skyzh mentioned this pull request Mar 18, 2022

feat(ci): use nextest risingwavelabs/risingwave#1048

Merged

2 tasks

feat: add cargo nextest support

1b0d581

Signed-off-by: Alex Chi <iskyzh@gmail.com>

skyzh force-pushed the skyzh/add-support-nextest branch from 180e623 to 1b0d581 Compare March 18, 2022 00:35

disable doctest for nextest

7185a10

Signed-off-by: Alex Chi <iskyzh@gmail.com>

taiki-e approved these changes Mar 18, 2022

View reviewed changes

bors bot merged commit e7dbbde into taiki-e:main Mar 18, 2022

skyzh deleted the skyzh/add-support-nextest branch March 18, 2022 14:20

taiki-e mentioned this pull request Mar 18, 2022

Add simple test for cargo llvm-cov nextest #145

Merged

bors bot added a commit that referenced this pull request Mar 18, 2022

Merge #145

988def2

145: Add simple test for `cargo llvm-cov nextest` r=taiki-e a=taiki-e follow-up #144 Co-authored-by: Taiki Endo <te316e89@gmail.com>

bors bot added a commit that referenced this pull request Mar 18, 2022

Merge #145

3d29799

145: Add simple test for `cargo llvm-cov nextest` r=taiki-e a=taiki-e follow-up #144 Co-authored-by: Taiki Endo <te316e89@gmail.com>

taiki-e mentioned this pull request Apr 11, 2022

llvm-cov and nextest arguments in "inverted" positions #151

Closed

taiki-e mentioned this pull request Jul 21, 2022

Switch from clap to lexopt #197

Merged

bors bot added a commit that referenced this pull request Sep 6, 2022

Merge #197

9b06c2c

197: Switch from clap to lexopt r=taiki-e a=taiki-e Fixes #151, #144 (comment), and #144 (comment) Co-authored-by: Taiki Endo <te316e89@gmail.com>

kjetilkjeka mentioned this pull request Sep 7, 2022

Is it impossible to use nextest profiles? #216

Closed

taiki-e mentioned this pull request Mar 30, 2023

Use clap #250

Closed

taiki-e added the A-nextest Area: nextest integration https://github.com/nextest-rs/nextest label Apr 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add cargo nextest support #144

feat: add cargo nextest support #144

skyzh commented Mar 17, 2022 •

edited

Loading

taiki-e left a comment •

edited

Loading

taiki-e Mar 17, 2022

sunshowers Mar 17, 2022

skyzh Mar 17, 2022

skyzh commented Mar 18, 2022 •

edited

Loading

taiki-e left a comment

bors bot commented Mar 18, 2022

taiki-e commented Mar 18, 2022

sunshowers commented Mar 18, 2022 •

edited

Loading

taiki-e commented Mar 18, 2022

sunshowers commented Mar 18, 2022

sunshowers commented Mar 18, 2022 •

edited

Loading

sunshowers commented Mar 18, 2022

skyzh commented Mar 19, 2022

sunshowers commented Mar 19, 2022

taiki-e commented Mar 19, 2022

taiki-e commented Sep 10, 2022

feat: add cargo nextest support #144

feat: add cargo nextest support #144

Conversation

skyzh commented Mar 17, 2022 • edited Loading

taiki-e left a comment • edited Loading

Choose a reason for hiding this comment

taiki-e Mar 17, 2022

Choose a reason for hiding this comment

sunshowers Mar 17, 2022

Choose a reason for hiding this comment

skyzh Mar 17, 2022

Choose a reason for hiding this comment

skyzh commented Mar 18, 2022 • edited Loading

taiki-e left a comment

Choose a reason for hiding this comment

bors bot commented Mar 18, 2022

taiki-e commented Mar 18, 2022

sunshowers commented Mar 18, 2022 • edited Loading

taiki-e commented Mar 18, 2022

sunshowers commented Mar 18, 2022

sunshowers commented Mar 18, 2022 • edited Loading

sunshowers commented Mar 18, 2022

skyzh commented Mar 19, 2022

sunshowers commented Mar 19, 2022

taiki-e commented Mar 19, 2022

taiki-e commented Sep 10, 2022

skyzh commented Mar 17, 2022 •

edited

Loading

taiki-e left a comment •

edited

Loading

skyzh commented Mar 18, 2022 •

edited

Loading

sunshowers commented Mar 18, 2022 •

edited

Loading

sunshowers commented Mar 18, 2022 •

edited

Loading