Skip to content

Conversation

@mgeisler
Copy link

@mgeisler mgeisler commented Apr 6, 2022

Before, the top-most Cargo.toml file was used. Now we use the nearest Cargo.toml file which contain a line saying [workspace].

This should avoid false positives when Cargo workspaces are nested inside each other: in this case, the inner package will have an empty [workspace] table.

@mgeisler
Copy link
Author

mgeisler commented Apr 6, 2022

Eh, ehm... This fails because the tests interact with each other: the first unit test will read the environment variable and cache it.

So this will need some other approach to be testable (an Integration test could work, but it doesn't have access to the private to_abs_ws_path function).

@bjorn3
Copy link

bjorn3 commented Apr 6, 2022

You could make an integration test which creates a file containing expect![], run it with the option to update the test expectation and then check if it is correctly updated.

Before, the top-most `Cargo.toml` file was used. Now we use the
nearest `Cargo.toml` file which contain a line saying `[workspace]`.

This should avoid false positives when Cargo workspaces are nested
inside each other: in this case, the inner package will have an empty
`[workspace]` table.
@mgeisler mgeisler force-pushed the check-for-workspace branch from 9960b05 to 2d8af43 Compare April 6, 2022 12:17
@mgeisler
Copy link
Author

mgeisler commented Apr 6, 2022

You could make an integration test which creates a file containing expect![], run it with the option to update the test expectation and then check if it is correctly updated.

Ah, that's a good idea! I was shying away from that approach since I feared that I would need to call cargo test or similar... But I see now that I can trigger it all from the integration test. I've updated the commit accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants