Skip to content

bin not found causes "ignored" not an error #105

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

Open
sourcefrog opened this issue Jun 26, 2022 · 12 comments · Fixed by #148
Open

bin not found causes "ignored" not an error #105

sourcefrog opened this issue Jun 26, 2022 · 12 comments · Fixed by #148
Labels
A-trycmd Area: trycmd package breaking-change bug Not as expected

Comments

@sourcefrog
Copy link

In trying to set up trycmd I had this md file:

```trycmd
$ cargo mutants --help
```

This gives a result of ignored.

With --nocapture I could find out:

running 1 test
[              trycmd::runner] 	Substitutions { vars: {"[CWD]": "/home/mbp/src/mutants"}, unused: {"[EXE]"} }
[              trycmd::runner] 	bin="cargo" not found
[              trycmd::runner] 	Case: Ok(
    Output {
        path: "trycmd/hello.trycmd",
        id: Some(
            "4",
        ),
        spawn: Spawn {
            exit: None,
            status: Skipped,
        },
        stdout: None,
        stderr: None,
        fs: Filesystem {
            context: [],
        },
    },
)
Testing trycmd/hello.trycmd:4 ... ignored
test trycmd_tests ... ok

It seems to me that "file not found" should probably be a test failure (by default?) not ignored?

@epage
Copy link
Contributor

epage commented Jul 20, 2022

The idea was to gracefully allow mixing untestable examples with testable examples.

We do have the ignore tag and it is unlikely people will have testable and untestable examples within the same code block, so we could probably change this.

@sourcefrog
Copy link
Author

sourcefrog commented Jul 20, 2022 via email

@kleinesfilmroellchen
Copy link

It seems like I had the exact error, just that I couldn't even access detailed error information via --nocapture. This is definitely confusing as I first believed it to be a platform-specific error.

epage added a commit to epage/snapbox that referenced this issue Oct 6, 2022
This was originally written this way to be a low effort way to ignore,
especially if you wanted a markdown block of code that interspersed
different commands.

In the general case, though, you can ignore the block and this makes it
harder to root cause problems.

Fixes assert-rs#105
@epage epage closed this as completed in #148 Oct 6, 2022
@epage
Copy link
Contributor

epage commented Oct 6, 2022

0.14.0 is released with this changed.

@epage
Copy link
Contributor

epage commented Nov 3, 2022

I had forgotten: this is important for when examples are conditionally present: clap-rs/clap#4439

@kleinesfilmroellchen
Copy link

Interesting; is there a better solution that covers both use cases?

@AucaCoyan
Copy link

Hi! I'm a bit new to the project. What do you thing about returning an error if a binary is either skipped or doesn't have a name?
The ignored keyword tells my common sense that I wrote a #[skip] on purpose or I forgot to include in a glob path like tests/**/*.toml.
Having a console output "test is ignored" but not telling me why was a hard thing to debug in my case

@epage
Copy link
Contributor

epage commented Feb 20, 2024

I'd recommend reviewing the thread for what has been previously considered.

@epage
Copy link
Contributor

epage commented Apr 23, 2025

@veeso in #386 proposes making failing an opt-in with TestCases::set_fail_on_unresolved_bin.

This allows for both behaviors to be present and even allows us to delay making this a breaking change.

Based on the current design, a set_ prefix wouldn't be used. I lean towards this being unknown rather than unresolved (as we don't resolve binary names to paths). So an alternative name could be TestCases::fail_on_unknown_bin(bool) or TestCases::fail_unknown_bins(bool)

Alternatives:

@veeso
Copy link

veeso commented Apr 25, 2025

I've renamed the function/params to fail_on_unknown_bin

@epage
Copy link
Contributor

epage commented Apr 25, 2025

@veeso what are your thoughts on the alternatives?

@veeso
Copy link

veeso commented Apr 25, 2025

I think using an enum with the rule could be good as well and could prevent breaking changes if one day we decide to extend this behaviour to new kinds.

A closure would be too complex imo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-trycmd Area: trycmd package breaking-change bug Not as expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants