-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat: cache latest fuels
version
#3186
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
CodSpeed Performance ReportMerging #3186 will degrade performances by 60.41%Comparing Summary
Benchmarks breakdown
|
f65abae
to
3ecfed6
Compare
Coverage Report:
Changed Files:
|
@@ -0,0 +1,25 @@ | |||
import { checkAndLoadCache, saveToCache } from './fuelsVersionCache'; | |||
|
|||
export const getLatestFuelsVersion = async (): Promise<string | undefined> => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we potentially document the throwable conditions here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if I follow you, can you share some pseudo code? What do you mean by documenting the throwable conditions?
@@ -0,0 +1,7 @@ | |||
import * as checkForUpdatesMod from '../../src/cli/utils/checkForAndDisplayUpdates'; | |||
|
|||
export const mockCheckForUpdates = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we always mock a successful resolution?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because as of now this helper for the mock is only used in places where we always expect this functionality to succeed or not run.
const doesVersionCacheExist = fs.existsSync(FUELS_VERSION_CACHE_FILE); | ||
|
||
if (doesVersionCacheExist) { | ||
const cachedVersion = fs.readFileSync(FUELS_VERSION_CACHE_FILE, 'utf-8'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we trim this?
const cachedVersion = fs.readFileSync(FUELS_VERSION_CACHE_FILE, 'utf-8'); | |
const cachedVersion = fs.readFileSync(FUELS_VERSION_CACHE_FILE, 'utf-8').trim(); |
const version = await getLatestFuelsVersion(); | ||
expect(fetchSpy).toHaveBeenCalled(); | ||
expect(version).toEqual('1.0.0'); | ||
expect(saveCacheSpy).toHaveBeenCalledWith('1.0.0'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think this relates - we should cover both success and failure conditions in my opinion.
fuels
version #3177Summary
This PR adds a basic local cache for the latest
fuels
version fetched by thefuels
CLI.Checklist