8000 fix: change skia-canvas to github release by fengmk2 · Pull Request #715 · cnpm/cnpmcore · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: change skia-canvas to github release #715

New issue

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

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 13, 2024
Merged

fix: change skia-canvas to github release #715

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
< 8000 div class="d-flex flex-justify-center">
Diff view
7 changes: 2 additions & 5 deletions config/binaries.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,12 +134,9 @@ const binaries = {
'skia-canvas': {
category: 'skia-canvas',
description: 'A canvas environment for Node',
type: BinaryType.NodePreGyp,
type: BinaryType.GitHub,
repo: 'samizdatco/skia-canvas',
distUrl: 'https://skia-canvas.s3.us-east-1.amazonaws.com',
options: {
requiredNapiVersions: true,
},
distUrl: 'https://github.com/samizdatco/skia-canvas/releases',
},
wrtc: {
category: 'wrtc',
Expand Down
56 changes: 56 additions & 0 deletions test/common/adapter/binary/GithubBinary.test.ts
< 10000 /div>
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ describe('test/common/adapter/binary/GithubBinary.test.ts', () => {
beforeEach(async () => {
binary = await app.getEggObject(GithubBinary);
});

describe('fetch()', () => {
it('should fetch root and subdir work', async () => {
const response = await TestUtil.readJSONFile(TestUtil.getFixtures('electron-releases.json'));
Expand All @@ -34,5 +35,60 @@ describe('test/common/adapter/binary/GithubBinary.test.ts', () => {
}
// console.log(result.items);
});

it('should fetch skia-canvas', async () => {
const response = await TestUtil.readJSONFile(TestUtil.getFixtures('skia-canvas-releases.json'));
app.mockHttpclient(/https:\/\/api\.github\.com\/repos\/samizdatco\/skia-canvas\/releases/, 'GET', {
data: response,
status: 200,
});
let result = await binary.fetch('/', 'skia-canvas');
assert(result);
assert(result.items.length > 0);
// console.log(JSON.stringify(result.items, null, 2));
let matchDir = false;
for (const item of result.items) {
assert(item.isDir === true);
if (item.name === 'v0.9.30/') {
matchDir = true;
}
}
assert(matchDir);

Comment on lines +45 to +57
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider using Array.prototype.some() for more efficient directory checking

While the current implementation works, we can improve its efficiency and readability by using Array.prototype.some() instead of a loop with a flag.

Consider refactoring the directory check as follows:

- let matchDir = false;
- for (const item of result.items) {
-   assert(item.isDir === true);
-   if (item.name === 'v0.9.30/') {
-     matchDir = true;
-   }
- }
- assert(matchDir);
+ assert(result.items.every(item => item.isDir === true));
+ assert(result.items.some(item => item.name === 'v0.9.30/'));

This change will make the code more concise and potentially more efficient, as it can short-circuit once a match is found.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let result = await binary.fetch('/', 'skia-canvas');
assert(result);
assert(result.items.length > 0);
// console.log(JSON.stringify(result.items, null, 2));
let matchDir = false;
for (const item of result.items) {
assert(item.isDir === true);
if (item.name === 'v0.9.30/') {
matchDir = true;
}
}
assert(matchDir);
let result = await binary.fetch('/', 'skia-canvas');
assert(result);
assert(result.items.length > 0);
// console.log(JSON.stringify(result.items, null, 2));
assert(result.items.every(item => item.isDir === true));
assert(result.items.some(item => item.name === 'v0.9.30/'));

result = await binary.fetch('/v0.9.24/', 'skia-canvas');
assert(result?.items.every(item => !/{.*}/.test(item.url)));

result = await binary.fetch('/v0.9.30/', 'skia-canvas');
assert(result);
assert(result.items.length > 0);
console.log(JSON.stringify(result.items, null, 2));
let matchFile1 = false;
let matchFile2 = false;
let matchFile3 = false;
for (const item of result.items) {
assert(item.isDir === false);
if (item.name === 'skia-canvas-v0.9.30-darwin-arm64.tar.gz') {
assert(item.date === '2024-08-26T18:04:13Z');
assert(item.size === 7547563);
assert.equal(item.url, 'https://github.com/samizdatco/skia-canvas/releases/download/v0.9.30/skia-canvas-v0.9.30-darwin-arm64.tar.gz');
matchFile1 = true;
}
if (item.name === 'skia-canvas-v0.9.30-linux-arm-glibc.tar.gz') {
assert(item.date === '2024-08-26T18:04:17Z');
assert(item.size === 8836353);
assert.equal(item.url, 'https://github.com/samizdatco/skia-canvas/releases/download/v0.9.30/skia-canvas-v0.9.30-linux-arm-glibc.tar.gz');
matchFile2 = true;
}
if (item.name === 'skia-canvas-v0.9.30-win32-x64.tar.gz') {
assert(item.date === '2024-08-26T18:04:29Z');
assert(item.size === 7497076);
assert.equal(item.url, 'https://github.com/samizdatco/skia-canvas/releases/download/v0.9.30/skia-canvas-v0.9.30-win32-x64.tar.gz');
matchFile3 = true;
}
}
assert(matchFile1);
assert(matchFile2);
assert(matchFile3);
Comment on lines +58 to +91
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

⚠️ Potential issue

Improve test efficiency and remove console logging

The test case for fetching specific version data is thorough, but there are a few areas for improvement:

  1. The assertion for v0.9.24 is good, but consider adding a comment explaining the reason for this check.
  2. The detailed checks for v0.9.30 use a similar loop-and-flag approach as before, which can be improved.
  3. There's a console.log statement that should be removed before merging to production.

Consider the following improvements:

  1. Add a comment explaining the v0.9.24 check:
// Ensure that item URLs don't contain curly braces, which could indicate an unresolved template
assert(result?.items.every(item => !/{.*}/.test(item.url)));
  1. Refactor the v0.9.30 checks using Array.prototype.find():
const files = [
  {
    name: 'skia-canvas-v0.9.30-darwin-arm64.tar.gz',
    date: '2024-08-26T18:04:13Z',
    size: 7547563,
    url: 'https://github.com/samizdatco/skia-canvas/releases/download/v0.9.30/skia-canvas-v0.9.30-darwin-arm64.tar.gz'
  },
  // ... add other files here
];

files.forEach(expectedFile => {
  const file = result.items.find(item => item.name === expectedFile.name);
  assert(file, `File ${expectedFile.name} not found`);
  assert.equal(file.date, expectedFile.date);
  assert.equal(file.size, expectedFile.size);
  assert.equal(file.url, expectedFile.url);
});
  1. Remove the console.log statement on line 64.

Remove the console.log statement on line 64 before merging to production.

});
});
});
57 changes: 0 additions & 57 deletions test/common/adapter/binary/N 8000 odePreGypBinary.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,63 +137,6 @@ describe('test/common/adapter/binary/NodePreGypBinary.test.ts', () => {
assert(matchFile3);
});

it('should fetch skia-canvas', async () => {
app.mockHttpclient('https://registry.npmjs.com/skia-canvas', 'GET', {
data: await TestUtil.readFixturesFile('registry.npmjs.com/skia-canvas.json'),
});
app.mockHttpclient('https://nodejs.org/dist/index.json', 'GET', {
data: await TestUtil.readFixturesFile('nodejs.org/site/index.json'),
});
let result = await binary.fetch('/', 'skia-canvas');
assert(result);
assert(result.items.length > 0);
// console.log(JSON.stringify(result.items, null, 2));
let matchDir = false;
for (const item of result.items) {
assert(item.isDir === true);
if (item.name === 'v0.9.30/') {
matchDir = true;
}
}
assert(matchDir);

result = await binary.fetch('/v0.9.24/', 'skia-canvas');
assert(result?.items.every(item => !/{.*}/.test(item.url)));

result = await binary.fetch('/v0.9.30/', 'skia-canvas');
assert(result);
assert(result.items.length > 0);
// console.log(JSON.stringify(result.items, null, 2));
let matchFile1 = false;
let matchFile2 = false;
let matchFile3 = false;
for (const item of result.items) {
assert(item.isDir === false);
assert.deepEqual(item.ignoreDownloadStatuses, [ 404, 403 ]);
if (item.name === 'darwin-arm64-napi-v6-unknown.tar.gz') {
assert(item.date === '2022-06-08T01:53:43.908Z');
assert(item.size === '-');
assert(item.url === 'https://skia-canvas.s3.us-east-1.amazonaws.com/v0.9.30/darwin-arm64-napi-v6-unknown.tar.gz');
matchFile1 = true;
}
if (item.name === 'linux-arm-napi-v6-glibc.tar.gz') {
assert(item.date === '2022-06-08T01:53:43.908Z');
assert(item.size === '-');
assert(item.url === 'https://skia-canvas.s3.us-east-1.amazonaws.com/v0.9.30/linux-arm-napi-v6-glibc.tar.gz');
matchFile2 = true;
}
if (item.name === 'win32-x64-napi-v6-unknown.tar.gz') {
assert(item.date === '2022-06-08T01:53:43.908Z');
assert(item.size === '-');
assert(item.url === 'https://skia-canvas.s3.us-east-1.amazonaws.com/v0.9.30/win32-x64-napi-v6-unknown.tar.gz');
matchFile3 = true;
}
}
assert(matchFile1);
assert(matchFile2);
assert(matchFile3);
});

it('should fetch wrtc', async () => {
app.mockHttpclient('https://registry.npmjs.com/wrtc', 'GET', {
data: await TestUtil.readFixturesFile('registry.npmjs.com/wrtc.json'),
Expand Down
Loading
Loading
0