Fix SubResourceIntegrityTests to report all errors

[Skrypt]

It gets really boring creating a commit per item failing.
I updated all of the assets for sha-5... something in the Vue 3 PR branch and everything fails.

Now this PR will report all of the assets that are failing including if the assets itself has a bad url.

[hishamco]

This could happen if you have an invalid SHA which makes sense to me, If the intend to report all the errors at the end I never mind, but we could log a warning message for each during the test

[Skrypt]

Yes, but it already does a console.log

See example of multiple failing asserts here:

https://github.com/OrchardCMS/OrchardCore/actions/runs/8248723901/job/22559650953?pr=14256

[hishamco]

Can I push commit to your PR if you don't mind, then feel free to ignore it if you don't like it

[hishamco]

@Skrypt seems the SRI unit test helps you a lot for discovering invalid SHAs in your VueJS PR :)

[Skrypt]

Yeah, but these are valid SHAs just not sha384 which cause issues. Maybe it could be more intelligent and do a split on the string to find which SHA to generate and compare with.

[hishamco]

Maybe it could be more intelligent and do a split on the string to find which SHA to generate and compare with.

Maybe but while we are using SHA384 we should unify them

[hishamco]

I forgot @Skrypt, recently we did an update on this, and now you can run the SRI test in your development machine. So, maybe no need to report all the errors because all the current resources are up to date. The issue raised in your PR because it's old IMHO

For that I think there no changed needed and you could fix the SHA quickly without waiting for the CI to discover them

Side note: Add CI environment variable to make the test works ar your end

[Skrypt]

Yeah, but it is really boring to need to do a commit per SHA failing as it is currently only reporting one failing assert at a time. Everyone that will merge latest dev on their PR and that need to add assets to a resource file will face this issue. Last night, I did almost 15 commits because CI was reporting one SHA failing at a time which doesn't make sense to me.

Why then do we need to have a CI environment variable for this test to execute if it is mandatory for the CI to build?
I tried simply removing the CI attribute on the test and it did not work for me last night. The unit test was disapearing from the list. No idea why 🙈 😄

[Skrypt]

More on that. Maybe we need a small tool to refactor these files with proper SHA if the intent is to always only use SHA384. That be better than just rely on testing and copy/pasting SHA from the command line.

[Skrypt]

Testing if file not found first.

[hishamco]

Why then do we need to have a CI environment variable for this test to execute if it is mandatory for the CI to build?

This is useful for running the test in development, but in the CI build we should do it on every PR, @sebastienros your thoughts if you see a value for this PR

[sebastienros]

Why then do we need to have a CI environment variable for this test to execute if it is mandatory for the CI to build?

99% of our commits won't change static assets that have this SHA. The 1% left should update the assets and the SHAs. This way it's only run as a safe-guard, on the CI. If you really want to run these locally, and download all the css/js/+minified files every time you run the tests then define the ENV on your machine, but other users won't be impacted.

If that was just me we could even create a "Fact" that accepts a argument with an ENV name such that we could run these tests in a GH action with a CRON to not even run for all our PRs. But this could also be solved by have these tests in a separate assembly and pick which test assemblies we want to run and when. Maybe this would solve your problem, you could just select this assembly when you want to run it locally instead of setting an ENV.

[hishamco]

What if we target building such tests in the release branch to avoid running it on every PR, which makes sense because we need to ensure that SHAs are valid before a release

[Skrypt]

I see your point about downloading all the assets on CI every time. If we don't want to do this then we need something that can be triggered manually. I will just keep this test for myself for now. It is just easier than executing the tests 8 times to get a new hash to copy in the code.

I think we should have something like the Po extractor tool that would execute from CI and create an issue automatically with complete log of failing hashes. Could be a Github action triggered manually. Could create a Github issue or not.

But I definitely want to be able to execute this locally without needing to put another ENV variable locally.

[Skrypt]

It was still fast though to download and test them all from CI.

[Skrypt]

Closing as non-issue for CI.