-
Notifications
You must be signed in to change notification settings - Fork 39
Simplify electron autoupdater #1361
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
base: main
Are you sure you want to change the base?
Changes from all commits
fb20ea9
00d2e0a
480af77
40392dd
25e43e4
7e129c2
818afd7
23b0e49
bffe603
39dd64e
6b6f372
0730c92
9319504
e846494
5c96446
efc351e
300dda3
7158c99
28a145b
3c8e632
a9133a9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,7 @@ module.exports = { | |
| 'sonarjs/no-alphabetical-sort': 'off', | ||
| 'sonarjs/no-empty-test-file': 'off', | ||
| 'sonarjs/os-command': 'off', | ||
| 'sonarjs/publicly-writable-directories': 'off', | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dajimenezriv-internxt We should return it, it's a useful check. We can always pass the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The eslint rule doesn't apply there, it's just for the test files (https://github.com/internxt/drive-desktop/pull/1361/changes#diff-e2954b558f2aa82baff0e30964490d12942e0e251c1aa56c3294de6ec67b7cf5L21) because I'm writing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Never mind, you are right, it's for tests only |
||
| }, | ||
| }, | ||
| ], | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,8 +7,9 @@ on: | |
| jobs: | ||
| check_pr_size: | ||
| # Ignore PRs with name merge-X-X-X-release | ||
| if: | | ||
| !(startsWith(github.head_ref, 'merge-') && endsWith(github.head_ref, '-release') && github.base_ref == 'main') | ||
| # if: | | ||
| # !(startsWith(github.head_ref, 'merge-') && endsWith(github.head_ref, '-release') && github.base_ref == 'main') | ||
| if: false | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the PR size checker turned off for all?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, because there are many tests and the lines were 550, to not split a test into another PR. It will be reverted in the next PR. |
||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 1 | ||
|
|
||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,5 +36,5 @@ $base64 = [System.Convert]::ToBase64String($bytes) | |
| Write-Host "Exe base64 hash: $base64" | ||
|
|
||
| (Get-Content $yamlPath) ` | ||
| | ForEach-Object { $_ -replace '^(\s*sha512:\s*).+', "`$1$base64" } ` | ||
| | ForEach-Object { $_ -replace '^(\s*sha512:\s*).+', ('${1}' + $base64) } ` | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The build release script was sometimes generating wrongly the |
||
| | Set-Content $yamlPath | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| import * as access from '@/infra/file-system/services/access'; | ||
| import { loggerFn } from '@/tests/vitest/mocks.helper.test'; | ||
| import { call, calls, partialSpyOn, TestProps } from '@/tests/vitest/utils.helper.test'; | ||
| import { checkAndInstall } from './check-and-install'; | ||
| import * as showDialog from './show-dialog'; | ||
| import * as verifyHashModule from './verify-hash'; | ||
|
|
||
| describe('check-and-install', () => { | ||
| const existsSyncMock = partialSpyOn(access, 'access'); | ||
| const verifyHashMock = partialSpyOn(verifyHashModule, 'verifyHash'); | ||
| const installReleaseMock = partialSpyOn(showDialog, 'installRelease'); | ||
|
|
||
| const props: TestProps<typeof checkAndInstall> = { | ||
| filePath: '/tmp/Internxt-Setup-10.0.0.exe', | ||
| }; | ||
|
|
||
| beforeEach(() => { | ||
| existsSyncMock.mockResolvedValue(undefined); | ||
| installReleaseMock.mockReturnValue(true); | ||
| }); | ||
|
|
||
| it('should skip if file does not exist', async () => { | ||
| // Given | ||
| existsSyncMock.mockResolvedValue(new access.AccessError('NON_EXISTS')); | ||
| // When | ||
| const res = await checkAndInstall(props as any); | ||
| // Then | ||
| expect(res).toBeUndefined(); | ||
| calls(verifyHashMock).toHaveLength(0); | ||
| }); | ||
|
|
||
| it('should install release if file exists and hash is valid', async () => { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add a test for trying to install an inferior version to what the user already has? Say the user has 11.0.0 and we try to install 10.0.0
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It cannot reach this code, as explained in the above comment. |
||
| // When | ||
| const res = await checkAndInstall(props as any); | ||
| // Then | ||
| expect(res).toBe(true); | ||
| calls(verifyHashMock).toHaveLength(1); | ||
| calls(installReleaseMock).toHaveLength(1); | ||
| call(loggerFn).toStrictEqual({ msg: 'Release already downloaded' }); | ||
| }); | ||
|
|
||
| it('should skip install if hash verification fails', async () => { | ||
| // Given | ||
| verifyHashMock.mockRejectedValue(new Error('sha512 mismatch')); | ||
| // When | ||
| const res = await checkAndInstall(props as any); | ||
| // Then | ||
| expect(res).toBeUndefined(); | ||
| calls(installReleaseMock).toHaveLength(0); | ||
| calls(loggerFn).toMatchObject([{ msg: 'Release already downloaded' }, { msg: 'Invalid downloaded release' }]); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| import { logger } from '@internxt/drive-desktop-core/build/backend'; | ||
| import { access } from '@/infra/file-system/services/access'; | ||
| import { installRelease } from './show-dialog'; | ||
| import { verifyHash } from './verify-hash'; | ||
|
|
||
| export async function checkAndInstall({ filePath }: { filePath: string }) { | ||
| const error = await access(filePath); | ||
| if (error) return; | ||
|
|
||
| try { | ||
| logger.debug({ msg: 'Release already downloaded' }); | ||
| await verifyHash({ filePath }); | ||
| return installRelease({ filePath }); | ||
| } catch (error) { | ||
| logger.debug({ msg: 'Invalid downloaded release', error }); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| import { app } from 'electron'; | ||
| import { loggerFn } from '@/tests/vitest/mocks.helper.test'; | ||
| import { call, calls, partialSpyOn } from '@/tests/vitest/utils.helper.test'; | ||
| import * as checkAndInstall from './check-and-install'; | ||
| import { checkForUpdates, isNewer } from './check-for-updates'; | ||
| import * as downloadRelease from './download-release'; | ||
|
|
||
| describe('check-for-updates', () => { | ||
| const appMock = vi.mocked(app); | ||
| const checkAndInstallMock = partialSpyOn(checkAndInstall, 'checkAndInstall'); | ||
| const downloadReleaseMock = partialSpyOn(downloadRelease, 'downloadRelease'); | ||
|
|
||
| beforeEach(() => { | ||
| vi.useFakeTimers(); | ||
| vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ json: vi.fn().mockResolvedValue({ name: '10.0.0' }) })); | ||
|
|
||
| // @ts-expect-error is read only | ||
| app.isPackaged = true; | ||
| }); | ||
|
|
||
| it('should skip if app is not packaged', async () => { | ||
| // Given | ||
| // @ts-expect-error is read only | ||
| appMock.isPackaged = false; | ||
| // When | ||
| const res = await checkForUpdates(); | ||
| // Then | ||
| expect(res).toBeUndefined(); | ||
| calls(fetch).toHaveLength(0); | ||
| }); | ||
|
|
||
| it('should throw error if release schema is invalid', async () => { | ||
| // Given | ||
| vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ json: vi.fn().mockResolvedValue({ name: '2.0' }) })); | ||
| // When | ||
| const res = await checkForUpdates(); | ||
| // Then | ||
| expect(res).toBeUndefined(); | ||
| call(loggerFn).toMatchObject({ | ||
| msg: 'Check for updates failed', | ||
| error: { | ||
| message: expect.stringContaining('name must match X.X.X'), | ||
| }, | ||
| }); | ||
| }); | ||
|
|
||
| it('should check new releases every hour if app is up to date', async () => { | ||
| // Given | ||
| vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ json: vi.fn().mockResolvedValue({ name: '2.0.0' }) })); | ||
| // When | ||
| const res = await checkForUpdates(); | ||
| // Then | ||
| expect(res).toBeUndefined(); | ||
| call(fetch).toBe('https://api.github.com/repos/internxt/drive-desktop/releases/latest'); | ||
| call(loggerFn).toStrictEqual({ msg: 'App is up to date', latest: '2.0.0' }); | ||
| // When | ||
| vi.advanceTimersByTime(60 * 60 * 1000); | ||
| // Then | ||
| calls(fetch).toHaveLength(2); | ||
| }); | ||
|
|
||
| it('should not download release if it was already downloaded', async () => { | ||
| // Given | ||
| checkAndInstallMock.mockResolvedValue(true); | ||
| // When | ||
| const res = await checkForUpdates(); | ||
| // Then | ||
| expect(res).toBe(true); | ||
| calls(downloadReleaseMock).toHaveLength(0); | ||
| }); | ||
|
|
||
| it('should download release if it was not downloaded', async () => { | ||
| // Given | ||
| checkAndInstallMock.mockResolvedValue(false); | ||
| // When | ||
| const res = await checkForUpdates(); | ||
| // Then | ||
| expect(res).toBeUndefined(); | ||
| call(downloadReleaseMock).toMatchObject({ fileName: 'Internxt-Setup-10.0.0.exe', latest: '10.0.0' }); | ||
| }); | ||
|
|
||
| it('isNewer', () => { | ||
| // equal | ||
| expect(isNewer('1.0.0', '1.0.0')).toBe(false); | ||
| // major | ||
| expect(isNewer('1.0.0', '2.0.0')).toBe(true); | ||
| expect(isNewer('2.0.0', '1.0.0')).toBe(false); | ||
| // minor | ||
| expect(isNewer('1.0.0', '1.1.0')).toBe(true); | ||
| expect(isNewer('1.1.0', '1.0.0')).toBe(false); | ||
| // patch | ||
| expect(isNewer('1.0.0', '1.0.1')).toBe(true); | ||
| expect(isNewer('1.0.1', '1.0.0')).toBe(false); | ||
| // major dominates minor/patch | ||
| expect(isNewer('1.9.9', '2.0.0')).toBe(true); | ||
| expect(isNewer('2.0.0', '1.9.9')).toBe(false); | ||
| // minor dominates patch | ||
| expect(isNewer('1.0.9', '1.1.0')).toBe(true); | ||
| expect(isNewer('1.1.0', '1.0.9')).toBe(false); | ||
| }); | ||
| }); |
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.
@dajimenezriv-internxt It's not fixing the Sonar warnings; it just disables them. Sonar keeps complaining but silently :)
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.
It's because we are using the test path /tmp inside the tests, however the rule is only disabled inside the tests. The warnings inside the production code are not disabled.
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.
https://github.com/internxt/drive-desktop/pull/1361/changes/BASE..e8464948dd462cae74b1bf650b8da0dd25dcbb08#diff-e2954b558f2aa82baff0e30964490d12942e0e251c1aa56c3294de6ec67b7cf5L21