Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Package Test Cache Issue #302

Merged
merged 2 commits into from
Jan 6, 2023
Merged

Conversation

confused-Techie
Copy link
Member

@confused-Techie confused-Techie commented Jan 5, 2023

While originally this PR set out to create a new package test runner script, I've ditched the idea as listing each core package as it's own test in unwieldy and overall more difficult to manage.

So instead I've gone ahead and just resolved our caching issues.

Previously our ppm and node_modules cache used a hash of the file, but the inside of a package can change with no change to the file itself, leaving old caches being used on package tests which would mean changes to packages were not present in the test, which could lead to false positives, and false negatives on error status.

So instead the cache now uses the Commit SHA for it's name. Meaning the cache can still be used per commit, providing some speed advantage, but will now accurately reflect the changes within each package.

@confused-Techie confused-Techie changed the title WIP - Test Verbose Package Test Runner in CI Fix Package Test Cache Issue Jan 5, 2023
@mauricioszabo
Copy link
Contributor

Honestly, I would prefer to have a cache if anything changed at all, like, if some packages/ contents changed, or if a yarn lockfile changed, etc.

Realistically, I don't trust GitHub to do the right thing nowadays, so I will go with your code :)

@confused-Techie
Copy link
Member Author

Honestly, I would prefer to have a cache if anything changed at all, like, if some packages/ contents changed, or if a yarn lockfile changed, etc.

Realistically, I don't trust GitHub to do the right thing nowadays, so I will go with your code :)

I'd personally be more enthusiastic about checking for any content changes within the contents of packages, since I worry we still might miss changes trusting lock files.

And while we can go with this now, I could play around with the idea of adding a small script to our scripts that could be run on GitHub actions that will generate a hash of the content of all our packages and use that as the name for our cache of npm and apm. Would be a lot nicer to GitHub and would be a proper cache of the contents rather than what I have here.

But thanks for the approval, I'll go ahead and merge this one, and we can find the best solution at a later time, for now this gets the job done enough to be able to rely on the output of our package tests.

@confused-Techie confused-Techie merged commit 1ade1ba into master Jan 6, 2023
@Spiker985 Spiker985 deleted the verbose-package-tests branch February 24, 2023 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants