From 797a937cd0f5bd2abc2dfb59ae2c205e9e786aad Mon Sep 17 00:00:00 2001 From: Akos Kitta Date: Thu, 8 Jun 2023 17:50:44 +0200 Subject: [PATCH 1/2] test: run cloud sketches tests on the CI - fix(test): integration tests are more resilient. - run the Create integration tests with other slow tests, - queued `PUT`/`DELETE` requests to make the test timeout happy, - reduced the `/sketches/search` offset to 1/5th and - remove Create API logging. - fix(linter): ignore `lib` folder. Remove obsolete `.node_modules` pattern. - feat(ci): enable Create API integration tests. Signed-off-by: Akos Kitta --- .eslintrc.js | 2 +- .github/workflows/build.yml | 3 + .../src/browser/create/create-api.ts | 20 +--- .../src/browser/create/typings.ts | 7 ++ ...te-api.test.ts => create-api.slow-test.ts} | 97 +++++++++++++++---- 5 files changed, 92 insertions(+), 37 deletions(-) rename arduino-ide-extension/src/test/browser/{create-api.test.ts => create-api.slow-test.ts} (83%) diff --git a/.eslintrc.js b/.eslintrc.js index 603ad7886..da16cbb76 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -10,7 +10,6 @@ module.exports = { ignorePatterns: [ 'node_modules/*', '**/node_modules/*', - '.node_modules/*', '.github/*', '.browser_modules/*', 'docs/*', @@ -21,6 +20,7 @@ module.exports = { '!electron-app/webpack.config.js', 'plugins/*', 'arduino-ide-extension/src/node/cli-protocol', + '**/lib/*', ], settings: { react: { diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index a444f83c0..2929add02 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -120,6 +120,9 @@ jobs: IS_NIGHTLY: ${{ github.event_name == 'schedule' || (github.event_name == 'workflow_dispatch' && github.ref == 'refs/heads/main') }} IS_RELEASE: ${{ startsWith(github.ref, 'refs/tags/') }} CAN_SIGN: ${{ secrets[matrix.config.certificate-secret] != '' }} + CREATE_USERNAME: ${{ secrets.CREATE_USERNAME }} + CREATE_PASSWORD: ${{ secrets.CREATE_PASSWORD }} + CREATE_CLIENT_SECRET: ${{ secrets.CREATE_CLIENT_SECRET }} run: | # See: https://www.electron.build/code-signing if [ $CAN_SIGN = false ]; then diff --git a/arduino-ide-extension/src/browser/create/create-api.ts b/arduino-ide-extension/src/browser/create/create-api.ts index 89244a427..7f4cfed22 100644 --- a/arduino-ide-extension/src/browser/create/create-api.ts +++ b/arduino-ide-extension/src/browser/create/create-api.ts @@ -179,7 +179,8 @@ export class CreateApi { ); }) .catch((reason) => { - if (reason?.status === 404) return [] as Create.Resource[]; + if (reason?.status === 404) + return [] as Create.Resource[]; // TODO: must not swallow 404 else throw reason; }); } @@ -486,18 +487,12 @@ export class CreateApi { await this.run(url, init, ResponseResultProvider.NOOP); } - private fetchCounter = 0; private async run( requestInfo: URL, init: RequestInit | undefined, resultProvider: ResponseResultProvider = ResponseResultProvider.JSON ): Promise { - const fetchCount = `[${++this.fetchCounter}]`; - const fetchStart = performance.now(); - const method = init?.method ? `${init.method}: ` : ''; - const url = requestInfo.toString(); const response = await fetch(requestInfo.toString(), init); - const fetchEnd = performance.now(); if (!response.ok) { let details: string | undefined = undefined; try { @@ -508,18 +503,7 @@ export class CreateApi { const { statusText, status } = response; throw new CreateError(statusText, status, details); } - const parseStart = performance.now(); const result = await resultProvider(response); - const parseEnd = performance.now(); - console.debug( - `HTTP ${fetchCount} ${method}${url} [fetch: ${( - fetchEnd - fetchStart - ).toFixed(2)} ms, parse: ${(parseEnd - parseStart).toFixed( - 2 - )} ms] body: ${ - typeof result === 'string' ? result : JSON.stringify(result) - }` - ); return result; } diff --git a/arduino-ide-extension/src/browser/create/typings.ts b/arduino-ide-extension/src/browser/create/typings.ts index fe2ca1af8..b5dcf6f59 100644 --- a/arduino-ide-extension/src/browser/create/typings.ts +++ b/arduino-ide-extension/src/browser/create/typings.ts @@ -82,6 +82,13 @@ export function isNotFound(err: unknown): err is NotFoundError { return isErrorWithStatusOf(err, 404); } +export type UnprocessableContentError = CreateError & { status: 422 }; +export function isUnprocessableContent( + err: unknown +): err is UnprocessableContentError { + return isErrorWithStatusOf(err, 422); +} + function isErrorWithStatusOf( err: unknown, status: number diff --git a/arduino-ide-extension/src/test/browser/create-api.test.ts b/arduino-ide-extension/src/test/browser/create-api.slow-test.ts similarity index 83% rename from arduino-ide-extension/src/test/browser/create-api.test.ts rename to arduino-ide-extension/src/test/browser/create-api.slow-test.ts index a9b52c11b..b5f4ee96c 100644 --- a/arduino-ide-extension/src/test/browser/create-api.test.ts +++ b/arduino-ide-extension/src/test/browser/create-api.slow-test.ts @@ -5,17 +5,24 @@ import { } from '@theia/core/shared/inversify'; import { assert, expect } from 'chai'; import fetch from 'cross-fetch'; +import { rejects } from 'node:assert'; import { posix } from 'node:path'; +import PQueue from 'p-queue'; +import queryString from 'query-string'; import { v4 } from 'uuid'; import { ArduinoPreferences } from '../../browser/arduino-preferences'; import { AuthenticationClientService } from '../../browser/auth/authentication-client-service'; import { CreateApi } from '../../browser/create/create-api'; import { splitSketchPath } from '../../browser/create/create-paths'; -import { Create, CreateError } from '../../browser/create/typings'; +import { + Create, + CreateError, + isNotFound, + isUnprocessableContent, +} from '../../browser/create/typings'; import { SketchCache } from '../../browser/widgets/cloud-sketchbook/cloud-sketch-cache'; import { SketchesService } from '../../common/protocol'; import { AuthenticationSession } from '../../node/auth/types'; -import queryString from 'query-string'; /* eslint-disable @typescript-eslint/no-non-null-asserted-optional-chain */ /* eslint-disable @typescript-eslint/no-non-null-assertion */ @@ -44,6 +51,11 @@ describe('create-api', () => { await cleanAllSketches(); }); + afterEach(async function () { + this.timeout(timeout); + await cleanAllSketches(); + }); + function createContainer(accessToken: string): Container { const container = new Container({ defaultScope: 'Singleton' }); container.load( @@ -120,13 +132,14 @@ describe('create-api', () => { async function cleanAllSketches(): Promise { let sketches = await createApi.sketches(); - // Cannot delete the sketches with `await Promise.all` as all delete promise successfully resolve, but the sketch is not deleted from the server. - await sketches - .map(({ path }) => createApi.deleteSketch(path)) - .reduce(async (acc, curr) => { - await acc; - return curr; - }, Promise.resolve()); + const deleteExecutionQueue = new PQueue({ + concurrency: 5, + autoStart: true, + }); + sketches.forEach(({ path }) => + deleteExecutionQueue.add(() => createApi.deleteSketch(path)) + ); + await deleteExecutionQueue.onIdle(); sketches = await createApi.sketches(); expect(sketches).to.be.empty; } @@ -229,8 +242,52 @@ describe('create-api', () => { expect(findByName(otherName, sketches)).to.be.not.undefined; }); + it('should error with HTTP 422 when reading a file but is a directory', async () => { + const name = v4(); + const content = 'void setup(){} void loop(){}'; + const posixPath = toPosix(name); + + await createApi.createSketch(posixPath, content); + const resources = await createApi.readDirectory(posixPath); + expect(resources).to.be.not.empty; + + await rejects(createApi.readFile(posixPath), (thrown) => + isUnprocessableContent(thrown) + ); + }); + + it('should error with HTTP 422 when listing a directory but is a file', async () => { + const name = v4(); + const content = 'void setup(){} void loop(){}'; + const posixPath = toPosix(name); + + await createApi.createSketch(posixPath, content); + const mainSketchFilePath = posixPath + posixPath + '.ino'; + const sketchContent = await createApi.readFile(mainSketchFilePath); + expect(sketchContent).to.be.equal(content); + + await rejects(createApi.readDirectory(mainSketchFilePath), (thrown) => + isUnprocessableContent(thrown) + ); + }); + + it("should error with HTTP 404 when deleting a non-existing directory via the '/files/d' endpoint", async () => { + const name = v4(); + const posixPath = toPosix(name); + + const sketches = await createApi.sketches(); + const sketch = findByName(name, sketches); + expect(sketch).to.be.undefined; + + await rejects(createApi.deleteDirectory(posixPath), (thrown) => + isNotFound(thrown) + ); + }); + ['.', '-', '_'].map((char) => { - it(`should create a new sketch with '${char}' in the sketch folder name although it's disallowed from the Create Editor`, async () => { + it(`should create a new sketch with '${char}' (character code: ${char.charCodeAt( + 0 + )}) in the sketch folder name although it's disallowed from the Create Editor`, async () => { const name = `sketch${char}`; const posixPath = toPosix(name); const newSketch = await createApi.createSketch( @@ -300,19 +357,23 @@ describe('create-api', () => { diff < 0 ? '<' : diff > 0 ? '>' : '=' } limit)`, async () => { const content = 'void setup(){} void loop(){}'; - const maxLimit = 50; // https://github.com/arduino/arduino-ide/pull/875 + const maxLimit = 10; const sketchCount = maxLimit + diff; const sketchNames = [...Array(sketchCount).keys()].map(() => v4()); - await sketchNames - .map((name) => createApi.createSketch(toPosix(name), content)) - .reduce(async (acc, curr) => { - await acc; - return curr; - }, Promise.resolve() as Promise); + const createExecutionQueue = new PQueue({ + concurrency: 5, + autoStart: true, + }); + sketchNames.forEach((name) => + createExecutionQueue.add(() => + createApi.createSketch(toPosix(name), content) + ) + ); + await createExecutionQueue.onIdle(); createApi.resetRequestRecording(); - const sketches = await createApi.sketches(); + const sketches = await createApi.sketches(maxLimit); const allRequests = createApi.requestRecording.slice(); expect(sketches.length).to.be.equal(sketchCount); From 4d58190f0e26a5651fca5883ebd06879ef044a3d Mon Sep 17 00:00:00 2001 From: Akos Kitta <1405703+kittaakos@users.noreply.github.com> Date: Fri, 6 Oct 2023 08:45:25 +0200 Subject: [PATCH 2/2] Update .github/workflows/build.yml Co-authored-by: per1234 --- .github/workflows/build.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 2929add02..3164dbd0c 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -120,6 +120,8 @@ jobs: IS_NIGHTLY: ${{ github.event_name == 'schedule' || (github.event_name == 'workflow_dispatch' && github.ref == 'refs/heads/main') }} IS_RELEASE: ${{ startsWith(github.ref, 'refs/tags/') }} CAN_SIGN: ${{ secrets[matrix.config.certificate-secret] != '' }} + # The CREATE_* environment vars are only used to run tests. These secrets are optional. Dependent tests will + # be skipped if not available. CREATE_USERNAME: ${{ secrets.CREATE_USERNAME }} CREATE_PASSWORD: ${{ secrets.CREATE_PASSWORD }} CREATE_CLIENT_SECRET: ${{ secrets.CREATE_CLIENT_SECRET }}