Fix workspace crates tests not being run

[Urgau]

Fixes #2391

[Urgau]

@Nemo157 This PR fixes the tests of the metadata and font-awesome-as-a-crate crates, it build them on CI and adds them to the CI cache (so that they can be run on the other jobs).

I saw that the CI jobs were run on my fork as well so as bonus I also restricted CI to only run on the master branch (when pushing)

[syphar]

thank you for working on this fix!
I have one question about the branch limitation.

Apart from that I'm actually not sure about the caching feature in general any more (which I added, so that's on me). It definitely improves CI times, but also it lead to us silently not running a part of the tests. And relying on test binary naming feels volatile at the least.

we could try to just use Swatinem/rust-cache in all jobs, and not share any binaries between the jobs. What do you think?

[syphar]

Also mentioned in the issue is that doctests aren't run, which in my mind is another reason to drop the artifact caching we are using and accept slower CI times before we have to rely on more cargo internals to keep the cache working.

[Urgau]

Apart from that I'm actually not sure about the caching feature in general any more (which I added, so that's on me). It definitely improves CI times, but also it lead to us silently not running a part of the tests. And relying on test binary naming feels volatile at the least

Yeah, relying on test binary names is not great. I tried using cargo --message-format but this felt overkill.

we could try to just use Swatinem/rust-cache in all jobs, and not share any binaries between the jobs. What do you think?

Yes, that would be great. I can try to do it, but won't have time to do it until a few days.
Maybe we can merge this PR in the mean time and follow-up with this bigger refactor.

[syphar]

Apart from that I'm actually not sure about the caching feature in general any more (which I added, so that's on me). It definitely improves CI times, but also it lead to us silently not running a part of the tests. And relying on test binary naming feels volatile at the least

Yeah, relying on test binary names is not great. I tried using cargo --message-format but this felt overkill.

we could try to just use Swatinem/rust-cache in all jobs, and not share any binaries between the jobs. What do you think?

Yes, that would be great. I can try to do it, but won't have time to do it until a few days. Maybe we can merge this PR in the mean time and follow-up with this bigger refactor.

This was broken for a long time, so having it broken for a little longer is not a big issue IMO.
So IMO we should directly do the refactor (or rather, simplification, refactor sounds so huge 😄 ).

[syphar]

last week I also started doing this (dropping the binary caching), though I didn't dig through the remaining test failures yet:

• master...syphar:docs.rs:fix-test
• https://github.com/syphar/docs.rs/actions/workflows/ci.yml?query=branch%3Afix-test

[Urgau]

I've updated the PR to remove the custom CI binary caching. I've consolidated the "build" and "build_test" jobs into a single "test" job. I also contains some tests fixes, but I did not fixed the ignored test since none are worth it, they mainly serve as copy/paste examples.

Let me if would like anything else.

@rustbot label -S-waiting-on-author +S-waiting-on-review

[Nemo157]

#[ignored] is really used as a way to categorize long-duration build-tests that we don't want to run locally normally, but they should be run in CI.

[Urgau]

#[ignored] is really used as a way to categorize long-duration build-tests that we don't want to run locally normally, but they should be run in CI.

Oops, forgot this. Fixed.

Note: CI failure is I believe transient (nope: it's been happening for at least 4 days): "not enough disk space"

[Urgau]

@Nemo157 @syphar What do you think about the latest changes? Can we move forward?

[syphar]

Sorry for the delay, vacation came in between.

Thank you for the contribution!