Add cache to node workflows

[oscard0m]

Summary
Follow up from #5895

[erezrokah]

Hi @oscard0m, comparing this PR workflow and an uncached one the CI times seems similar (if you sum up Use Node.js and install dependencies).

Can you confirm this? What are the benefits of adding cache except CI speed?

[oscard0m]

Hi @oscard0m, comparing this PR workflow and an uncached one the CI times seems similar (if you sum up Use Node.js and install dependencies).

Can you confirm this? What are the benefits of adding cache except CI speed?

There should not be any noticeable change in performance. setup-node's cache option uses actions/cache internally. I would say is just a syntax sugar to simplify workflows.

[erezrokah]

There should not be any noticeable change in performance. setup-node's cache option uses actions/cache internally. I would say is just a syntax sugar to simplify workflows.

To clarify, the workflow runs I linked compare a cached version via cache option vs a non cached version (see build job).

We do use caching for the e2e-with-cypress job since it only runs on Linux, and on that environment there is a small performance improvement to using a cache.

Should we apply this change only to the e2e-with-cypress job?

[oscard0m]

There should not be any noticeable change in performance. setup-node's cache option uses actions/cache internally. I would say is just a syntax sugar to simplify workflows.

To clarify, the workflow runs I linked compare a cached version via cache option vs a non cached version (see build job).

👍🏽

We do use caching for the e2e-with-cypress job since it only runs on Linux, and on that environment there is a small performance improvement to using a cache.

Should we apply this change only to the e2e-with-cypress job?

From what I read, it should be equivalent to use actions/cache or actions/setup-node with cache option:

• https://github.com/actions/setup-node#caching-packages-dependencies
• Clarify caching of npm / Yarn / pnpm dependencies actions/setup-node#304 (comment)

Here are some expected numbers depending on the Operating System and the number of dependencies: actions/setup-node#271 (comment)

Since, in my understanding, are equivalent, it would be a matter of reducing the amount of lines in this workflow.

On the other hand, I guess in the second scenario where we are running an extra step for checking out actions/cache can save us a few milliseconds but not sure of that.

Am I answering your doubts @erezrokah

[erezrokah]

Thanks @oscard0m, I think we are discussing 2 different things:

1. Should we replace action/cache with the setup-node cache option - I agree 💯 We can do that for the e2e-with-cypress job as it is already cached.

2. Should we add caching in general to jobs that don't have it? To answer that question I think it would help to see a similar comparison as in Support cache of dependencies in setup-node. actions/setup-node#271 (comment), just for this project.

Context: we removed caching for Windows a while back due to actions/cache#442

[oscard0m]

Thanks @oscard0m, I think we are discussing 2 different things:

Thanks for clarifying, yep, we were talking about 2 different things. Thanks for your patience here :)

1. Should we replace action/cache with the setup-node cache option - I agree 💯 We can do that for the e2e-with-cypress job as it is already cached.

👍🏽

2. Should we add caching in general to jobs that don't have it? To answer that question I think it would help to see a similar comparison as in Support cache of dependencies in setup-node. actions/setup-node#271 (comment), just for this project.

Context: we removed caching for Windows a while back due to actions/cache#442

I would be happy to apply these changes and run the comparison. Do you think it would have sense to do it in a different PR and change the title and commits to be more specific explaining the jobs affected?

[erezrokah]

Do you think it would have sense to do it in a different PR and change the title and commits to be more specific explaining the jobs affected?

I think it would. A separate PR for the thing we can ship and another to see performance changes will be great.

[erezrokah]

Closing this per our discussion. We can re-visit once we have a comparison on the performance improvements