Shouldn't these tests run regardless of whether the remote-checks feature is enabled? (it would be nice for cargo test to run all tests by default)

I like this as it is, because I don't like it when a test suite makes any more assumptions about the local environment than it absolutely needs to (and "I have a real, working, fully-open Internet connection" is still, in some cases, an incorrect assumption). I sometimes do dev without a working Internet connection, and a test suite that starts barfing red in all directions gives me the absolute pip.

In any event, I never trust developers (including myself!) to run all the tests before pushing (one cannot mandate that a pre-commit hook is in place in a remote repo, after all), so the most important thing is to make sure that CI is comprehensive -- which means, in this case, running the tests with remote-checks enabled.

It would be a nice "quality of life" thing if the test suite could print something at the end of a cargo test run saying "some tests weren't run because the remote-checks feature wasn't enabled", to give people a hint that re-running with extra features would be a good idea, but a lack of it wouldn't be a blocker for merge for me.

@mpalmer when you put it like that I completely agree :D

I see @mpalmer's point, however I made this decision for a different reason :) Interestingly, enabling remote-checks and running those first 9 tests resulted in different output for those tests! Hence why they were disabled for remote checks. We could alter those tests so they are idempotent to the issues they're testing, but I wanted to reduce the changes as much as I could.

This is a nitpick but it would maybe be clearer to rename this to something obviously wrong like actions/does-not-exist@v2. I had to re-read this a few times because I didn't spot the trailing "s" (possibly this is only an issue for me because I'm dyslexic!).

I'll update this in #48.

Currently, we are logging a warning to the console in the NPM version when we skip validating a glob. This was a bit of a hack since I didn't anticipate any other use cases for warnings. However, I'm wondering if it would make sense to add record warnings in the validation state, e.g.

#[derive(Serialize, Debug)]
pub struct ValidationState {
    // --snip--
    pub warnings: Vec<ValidationWarning>,
}

Then we could record and report a warning in this case.

Alternatively, we could also report this as an error but add a severity to errors e.g.

pub enum ErrorSeverity {
  Error,
  Warning,
}

I guess if we went with the latter approach it would make more sense to rename errors to diagnostics but probably best to avoid breaking API changes.

What do you think?

I'm OK with breaking API changes when we're early in the lifecycle, and I don't have strong opinions on either of these approaches, but it does seem like a good idea to be recording problems for later consumption, rather than logging anything immediately.

I think it would be good to add a new variant to the ValidationError enum for missign actions. Maybe something like ValidationError::UnresolvedAction.

Maybe we could use ValidationError::UnresolvedAction as suggested above, or if this is worth covering with it's own error code then perhaps something like ValidationError::InvalidActionFormat.

I like having very granular error codes, so I'm in team "InvalidActionFormat".