Test cargo lints

[ebroto]

changelog: Add infrastructure to test cargo lints

Closes #5603

[phansch]

Really happy to see tests for the cargo lints 🎉

Just a minor nit - looks good to me overall!

[flip1995]

LGTM overall. Maybe we should add a test (not necessarily in this PR) that ensures, that there are no stderr files in the pass dirs

[ebroto]

@flip1995 I think the test you mention is a good idea! But I've been thinking about a limitation of this implementation, that we could need in some cases to have multiple fail/pass manifests, for example if we want to test multiple values for the same attribute.

I plan to work on #5041 soon, I think it could be a good excuse to try to come up with a solution to the limitation I mentioned. Since this will probably have an impact in the filesystem layout, I think I could implement the test in that PR.

What do you think?

[flip1995]

that we could need in some cases to have multiple fail/pass manifests

Instead of checking [pass, fail], you could instead check for starts_with({pass,fail}), so directories fail-this-test will assume that the cargo lint will fail and pass-this-test will assume that the test passes.

On the other hand, why would they even have to be named pass or fail? Couldn't you just walk every subdir of the cargo tests? A missing stderr file would automatically indicate, that a test is pass.

[ebroto]

Couldn't you just walk every subdir of the cargo tests? A missing stderr file would automatically indicate, that a test is pass.

Indeed, that's much simpler, I like it 👍 I will make the changes needed and rework a bit the documentation.

Thanks!

[ebroto]

So I've applied the discussed changes. I went for leaving the fail/pass directories by default because I think it's good to enforce having at least one of each cases and IMO it's easier to understand for most of the cases.

I think the test is not viable anymore as the nomenclature is not enforced, let me know if you think otherwise.

[flip1995]

Having pass/fail as default is good.

I think the test is not viable anymore as the nomenclature is not enforced, let me know if you think otherwise.

Yes, with this, we won't need the test.

[flip1995]

Thanks!

@bors r+

[bors]

📌 Commit f9013ff has been approved by flip1995

[bors]

⌛ Testing commit f9013ff with merge 780572b...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing 780572b to master...