CI: Cache the packages/ directory (fixes broken language-rust-bundled test)

[DeeDeeG]

Ensures we do not miss restoring these packages' node_modules folders when running the test jobs.

Requirements for Contributing a Bug Fix (from template, click to expand):

• Fill out the template below. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
• The pull request must only fix an existing bug. To contribute other changes, you must use a different template. You can see all templates at https://github.com/atom/.github/tree/master/.github/PULL_REQUEST_TEMPLATE.
• The pull request must update the test suite to demonstrate the changed functionality. For guidance, please see https://flight-manual.atom.io/hacking-atom/sections/writing-specs/.
• After you create the pull request, all status checks must be pass before a maintainer reviews your contribution. For more details, please see https://github.com/atom/.github/tree/master/CONTRIBUTING.md#pull-requests.

Identify the Bug

Fixes an overlooked edge case pertaining to the Cache@2 CI task upgrade (#21057).

See discussion in #21790.

Due to the way npm installs packages that are specified as local paths (./some/path), and how caching was set up in Atom's CI, it was possible for Atom's CI to miss (fail to cache and restore) some packages' sub-dependencies. This could cause their tests to fail.

Description of the Change

In CI, start caching the packages/ directory. This ensures that those packages' node_modules folders (the packages' dependencies) are always restored in the post-build jobs. This is particularly relevant for ensuring the test jobs will pass.

Alternate Designs

• We could simply work around this infrequent issue by carefully hoisting dependencies. For example, this commit would solve the recent issue for the language-rust-bundled package: DeeDeeG@ab7c3b5

  • That would be a temporary solution though, meaning we might run into the same (rather confusing and counter-intuitive) issue again some time in the future.

• As an implementation detail, we could more easily specify these caches if using the older Lighthouse/Microsoft DevLabs caching tool: https://marketplace.visualstudio.com/items?itemName=1ESLighthouseEng.PipelineArtifactCaching

  • But this requires all forks of Atom wishing to run CI to install that addon task. Using the official Cache@V2 task keeps us current and makes it easier to fork Atom with working CI.

Possible Drawbacks

Adds about four or five seconds to each CI job.

Also, there will be some "warnings" about caches changing in the macOS Tests packages-2 job:

##[warning]The given cache key has changed in its resolved value between restore and save steps;

These warnings are expected and can be safely ignored, but they add noise to the CI runs. If I figure out exactly which files change (probably some test fixtures or temp test output?) I might be able to filter those out in the cache ID.

(As mentioned in "Alternative Designs", these warnings could be avoided with the old Lighthouse/Microsoft DevLabs cache task, but that task is unofficial and not supported anymore.)

Verification Process

Ran CI with "Enable system diagnostics" checked. The right files are now cached and restored, including the packages/ folder. See: CI link

Release Notes

N/A

[DeeDeeG]

@sadick254 this PR should prevent the kind of CI failures seen with language-rust-bundled recently (#21790).

Please take a look if you have time.

[sadick254]

Thank you for looking into this @DeeDeeG. Caching the packages folder is a better approach.

We could ignore the error for now. I would highly appreciate a follow-up PR to resolve the error.

##[warning]The given cache key has changed in its resolved value between restore and save steps;

[DeeDeeG]

[ Edit from the future: This was fixed by #21887 ]

Trying to get rid of those cache key has changed warnings...

I tried to figure out exactly what files are changing, but I could not locally reproduce the files changing after running tests...

(I bootstrapped the repository. Then did git add --force packages/ to stage the exact state of all the files under packages in git. Then I ran script/test --core-renderer and let it complete. Then I did git status --ignored -- packages/ and it didn't show me that any files had been changed. So... until I can figure out a way to deduce what files are changing, I'm not sure which ones to exclude (or if they are source files or spec fixtures, we may just have to live with the warnings, as caching them would be problematic.))

The most technically precise thing to do would be to individually cache the node_modules subfolder for each package under packages/ that has its own dependencies. But this is tedious to set up, very verbose in the cache.yml CI template, not very time-efficient on CI, and would break if any new monorepo packages were added, or if existing ones are updated to add their own dependencies. (We used to do all of this as the wildcard pattern **/node_modules, !**/node_modules/**/node_modules... But wildcard patters of directories to cache are not supported in the official Cache@V2 task.)

[DeeDeeG]

Linking to #21057 for findability.

This PR effectively fixed an overlooked edge case for the change in #21057.