From 917ae2c438bfbdd1c3c2725c9cdfccbf53175f99 Mon Sep 17 00:00:00 2001 From: Benjamin Weggersen Date: Fri, 25 Oct 2019 20:03:32 +0200 Subject: [PATCH] Fix Windows tests, cross-platform hasher and additional performance markers for fetching (#110) It looks like the CI agent for Windows got updated so now the Windows tests run and they are failing. This PR fixes that and adds support for cross-platform hashing. * Always store tsconfig.json files with LF newline * Update path joining and evaluation to support Windows * Change all newlines to LF to get the same hash across Windows and Posix --- .gitattributes | 1 + packages/backfill/src/__tests__/audit.test.ts | 2 +- .../backfill/src/__tests__/backfill.test.ts | 14 ++------- packages/backfill/src/__tests__/e2e.test.ts | 2 +- packages/backfill/src/audit.ts | 6 ++-- packages/backfill/src/commandRunner.ts | 2 +- packages/backfill/src/index.ts | 2 +- packages/cache/src/AzureBlobCacheStorage.ts | 14 +++++++-- packages/cache/src/CacheStorage.ts | 2 ++ packages/hasher/package.json | 4 ++- .../src/__tests__/getYarnWorkspaces.test.ts | 9 ++++-- packages/hasher/src/hashOfFiles.ts | 29 ++++++++++++------- .../basic-without-lock-file/package.json | 2 +- .../__fixtures__/basic/package.json | 2 +- .../monorepo/packages/package-a/package.json | 4 +-- .../monorepo/packages/package-b/package.json | 2 +- .../__fixtures__/with-cache/package.json | 2 +- yarn.lock | 5 ++++ 18 files changed, 63 insertions(+), 41 deletions(-) create mode 100644 .gitattributes diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..e09675f --- /dev/null +++ b/.gitattributes @@ -0,0 +1 @@ +tsconfig.json text eol=lf \ No newline at end of file diff --git a/packages/backfill/src/__tests__/audit.test.ts b/packages/backfill/src/__tests__/audit.test.ts index 81adf38..8d03fb8 100644 --- a/packages/backfill/src/__tests__/audit.test.ts +++ b/packages/backfill/src/__tests__/audit.test.ts @@ -44,6 +44,6 @@ describe("Audit", () => { ); expect(backfillOutput.all).toMatch(sideEffectWarningString); - expect(backfillOutput.all).toMatch("packages/DONE"); + expect(backfillOutput.all).toMatch(path.join("packages", "DONE")); }); }); diff --git a/packages/backfill/src/__tests__/backfill.test.ts b/packages/backfill/src/__tests__/backfill.test.ts index 9a7adf5..ee4b238 100644 --- a/packages/backfill/src/__tests__/backfill.test.ts +++ b/packages/backfill/src/__tests__/backfill.test.ts @@ -41,12 +41,7 @@ describe("backfill", () => { const spiedHasher = spy(hasher); // Execute - await backfill( - { ...config, producePerformanceLogs: false }, - cacheStorage, - spiedBuildCommand, - hasher - ); + await backfill(config, cacheStorage, spiedBuildCommand, hasher); // Assert verify(spiedHasher.createPackageHash()).once(); @@ -59,12 +54,7 @@ describe("backfill", () => { jest.clearAllMocks(); // Execute - await backfill( - { ...config, producePerformanceLogs: false }, - cacheStorage, - buildCommand, - hasher - ); + await backfill(config, cacheStorage, buildCommand, hasher); // Assert verify(spiedHasher.createPackageHash()).once(); diff --git a/packages/backfill/src/__tests__/e2e.test.ts b/packages/backfill/src/__tests__/e2e.test.ts index b921a3b..7e94803 100644 --- a/packages/backfill/src/__tests__/e2e.test.ts +++ b/packages/backfill/src/__tests__/e2e.test.ts @@ -20,7 +20,7 @@ describe("End to end", () => { // Verify it produces the correct hash const ownHash = fs.readdirSync(path.join(packageRoot, hashPath)); - expect(ownHash).toContain("f7c957c5f7a104d7ca6c176ad8a034d0512b5178"); + expect(ownHash).toContain("57f26541cc848f71a80fd9039137f1d50e013b92"); // ... and that `npm run compile` was run successfully const libFolderExist = await fs.pathExists("lib"); diff --git a/packages/backfill/src/audit.ts b/packages/backfill/src/audit.ts index c634f5f..f493a58 100644 --- a/packages/backfill/src/audit.ts +++ b/packages/backfill/src/audit.ts @@ -25,8 +25,8 @@ function getGitRepositoryRoot(packageRoot: string) { } function addGlobstars(globPatterns: string[]): string[] { - const folders = globPatterns.map(p => path.join("**", p, "**", "*")); - const files = globPatterns.map(p => path.join("**", p)); + const folders = globPatterns.map(p => path.posix.join("**", p, "**", "*")); + const files = globPatterns.map(p => path.posix.join("**", p)); return [...folders, ...files]; } @@ -58,7 +58,7 @@ export function initializeWatcher( ]); const cacheFolderGlob = outputFolderAsArray(outputFolder).map(folder => - path.join("**", folder, "**") + path.posix.join("**", folder, "**") ); watcher = chokidar diff --git a/packages/backfill/src/commandRunner.ts b/packages/backfill/src/commandRunner.ts index 82a6b17..7a4ea6f 100644 --- a/packages/backfill/src/commandRunner.ts +++ b/packages/backfill/src/commandRunner.ts @@ -45,7 +45,7 @@ export function createBuildCommand( // Catch to pretty-print the command that failed and re-throw .catch(err => { if (process.env.NODE_ENV !== "test") { - logger.error(`Failed while running: ${parsedBuildCommand}`); + logger.error(`Failed while running: "${parsedBuildCommand}"`); } throw err; }) diff --git a/packages/backfill/src/index.ts b/packages/backfill/src/index.ts index b257e5c..e9e9c78 100644 --- a/packages/backfill/src/index.ts +++ b/packages/backfill/src/index.ts @@ -40,7 +40,7 @@ export async function backfill( try { await buildCommand(); } catch (err) { - throw new Error("Command failed"); + throw new Error(`Command failed with the following error:\n\n${err}`); } try { diff --git a/packages/cache/src/AzureBlobCacheStorage.ts b/packages/cache/src/AzureBlobCacheStorage.ts index 0ca96d9..d4a8047 100644 --- a/packages/cache/src/AzureBlobCacheStorage.ts +++ b/packages/cache/src/AzureBlobCacheStorage.ts @@ -3,8 +3,11 @@ import * as fs from "fs-extra"; import * as tar from "tar"; import * as path from "path"; -import { AzureBlobCacheStorageOptions } from "backfill-config"; -import { outputFolderAsArray } from "backfill-config"; +import { logger } from "backfill-logger"; +import { + AzureBlobCacheStorageOptions, + outputFolderAsArray +} from "backfill-config"; import { CacheStorage } from "./CacheStorage"; @@ -53,6 +56,8 @@ export class AzureBlobCacheStorage extends CacheStorage { hash ); + logger.profile("cache:azure:download"); + const response = await blobClient.download(0); const blobReadableStream = response.readableStreamBody; @@ -60,12 +65,15 @@ export class AzureBlobCacheStorage extends CacheStorage { throw new Error("Unable to fetch blob."); } + logger.profile("cache:azure:download"); + fs.mkdirpSync(temporaryBlobOutputFolder); const tarWritableStream = tar.extract({ cwd: temporaryBlobOutputFolder }); + logger.profile("cache:azure:extract-to-temp"); blobReadableStream.pipe(tarWritableStream); const blobPromise = new Promise((resolve, reject) => { @@ -74,6 +82,8 @@ export class AzureBlobCacheStorage extends CacheStorage { }); await blobPromise; + + logger.profile("cache:azure:extract-to-temp"); } catch (error) { fs.removeSync(temporaryBlobOutputFolder); diff --git a/packages/cache/src/CacheStorage.ts b/packages/cache/src/CacheStorage.ts index 1a0133c..d54ebed 100644 --- a/packages/cache/src/CacheStorage.ts +++ b/packages/cache/src/CacheStorage.ts @@ -23,12 +23,14 @@ export abstract class CacheStorage implements ICacheStorage { return false; } + logger.profile("cache:fetch:copy-to-outputfolder"); await Promise.all( outputFolderAsArray(outputFolder).map(async folder => { await fs.mkdirp(folder); await fs.copy(path.join(localCacheFolder, folder), folder); }) ); + logger.profile("cache:fetch:copy-to-outputfolder"); logger.setHit(true); logger.setTime("fetchTime", "cache:fetch"); diff --git a/packages/hasher/package.json b/packages/hasher/package.json index 6e8f3ae..25ebb47 100644 --- a/packages/hasher/package.json +++ b/packages/hasher/package.json @@ -23,10 +23,12 @@ "fast-glob": "^3.0.4", "find-up": "^4.1.0", "find-yarn-workspace-root": "^1.2.1", - "fs-extra": "^8.1.0" + "fs-extra": "^8.1.0", + "normalize-path": "^3.0.0" }, "devDependencies": { "@types/fs-extra": "^8.0.0", + "@types/normalize-path": "^3.0.0", "@types/yarnpkg__lockfile": "^1.1.3", "backfill-utils-test": "^2.0.3", "backfill-utils-tsconfig": "^2.0.3", diff --git a/packages/hasher/src/__tests__/getYarnWorkspaces.test.ts b/packages/hasher/src/__tests__/getYarnWorkspaces.test.ts index ef1c38e..4008a4e 100644 --- a/packages/hasher/src/__tests__/getYarnWorkspaces.test.ts +++ b/packages/hasher/src/__tests__/getYarnWorkspaces.test.ts @@ -1,5 +1,6 @@ import * as path from "path"; import { setupFixture } from "backfill-utils-test"; +import normalize = require("normalize-path"); import { getYarnWorkspaces } from "../yarnWorkspaces"; @@ -8,8 +9,12 @@ describe("getYarnWorkspaces()", () => { const packageRoot = await setupFixture("monorepo"); const workspacesPackageInfo = getYarnWorkspaces(packageRoot); - const packageAPath = path.join(packageRoot, "packages", "package-a"); - const packageBPath = path.join(packageRoot, "packages", "package-b"); + const packageAPath = normalize( + path.join(packageRoot, "packages", "package-a") + ); + const packageBPath = normalize( + path.join(packageRoot, "packages", "package-b") + ); expect(workspacesPackageInfo).toEqual([ { name: "package-a", path: packageAPath }, diff --git a/packages/hasher/src/hashOfFiles.ts b/packages/hasher/src/hashOfFiles.ts index ab7c23b..d6f5422 100644 --- a/packages/hasher/src/hashOfFiles.ts +++ b/packages/hasher/src/hashOfFiles.ts @@ -3,6 +3,10 @@ import * as fg from "fast-glob"; import * as fs from "fs-extra"; import * as path from "path"; import { createConfig } from "backfill-config"; +import { hashStrings } from "./helpers"; + +const newline = /\r\n|\r|\n/g; +const LF = "\n"; export async function generateHashOfFiles( packageRoot: string @@ -17,17 +21,20 @@ export async function generateHashOfFiles( files.sort((a, b) => a.path.localeCompare(b.path)); - const hasher = crypto.createHash("sha1"); - files.forEach(file => { - hasher.update(file.path); + const hashes = await Promise.all( + files.map(async file => { + const hasher = crypto.createHash("sha1"); + hasher.update(file.path); - if (!file.dirent.isDirectory()) { - const data = fs - .readFileSync(path.join(packageRoot, file.path)) - .toString(); - hasher.update(data); - } - }); + if (!file.dirent.isDirectory()) { + const fileBuffer = await fs.readFile(path.join(packageRoot, file.path)); + const data = fileBuffer.toString().replace(newline, LF); + hasher.update(data); + } - return hasher.digest("hex"); + return hasher.digest("hex"); + }) + ); + + return hashStrings(hashes); } diff --git a/packages/utils-test/__fixtures__/basic-without-lock-file/package.json b/packages/utils-test/__fixtures__/basic-without-lock-file/package.json index 5948240..3b60c19 100644 --- a/packages/utils-test/__fixtures__/basic-without-lock-file/package.json +++ b/packages/utils-test/__fixtures__/basic-without-lock-file/package.json @@ -3,7 +3,7 @@ "license": "MIT", "version": "0.1.0", "scripts": { - "compile": "copy src/index.ts lib/index.js" + "compile": "node node_modules/.bin/copy src/index.ts lib/index.js" }, "dependencies": { "package-2": "*" diff --git a/packages/utils-test/__fixtures__/basic/package.json b/packages/utils-test/__fixtures__/basic/package.json index f0a1ba9..3110171 100644 --- a/packages/utils-test/__fixtures__/basic/package.json +++ b/packages/utils-test/__fixtures__/basic/package.json @@ -3,7 +3,7 @@ "license": "MIT", "version": "0.1.0", "scripts": { - "compile": "copy src/index.ts lib/index.js" + "compile": "node node_modules/.bin/copy src/index.ts lib/index.js" }, "dependencies": { "package-2": "*" diff --git a/packages/utils-test/__fixtures__/monorepo/packages/package-a/package.json b/packages/utils-test/__fixtures__/monorepo/packages/package-a/package.json index 33e57b1..fab1e73 100644 --- a/packages/utils-test/__fixtures__/monorepo/packages/package-a/package.json +++ b/packages/utils-test/__fixtures__/monorepo/packages/package-a/package.json @@ -3,7 +3,7 @@ "license": "MIT", "version": "0.1.0", "scripts": { - "compile": "copy src/index.ts lib/index.js", - "side-effect": "copy src/index.ts ../DONE" + "compile": "node node_modules/.bin/copy src/index.ts lib/index.js", + "side-effect": "node node_modules/.bin/copy src/index.ts ../DONE" } } diff --git a/packages/utils-test/__fixtures__/monorepo/packages/package-b/package.json b/packages/utils-test/__fixtures__/monorepo/packages/package-b/package.json index 7e151d6..8391385 100644 --- a/packages/utils-test/__fixtures__/monorepo/packages/package-b/package.json +++ b/packages/utils-test/__fixtures__/monorepo/packages/package-b/package.json @@ -3,6 +3,6 @@ "license": "MIT", "version": "0.1.0", "scripts": { - "compile": "copy src/index.ts lib/index.js" + "compile": "node node_modules/.bin/copy src/index.ts lib/index.js" } } diff --git a/packages/utils-test/__fixtures__/with-cache/package.json b/packages/utils-test/__fixtures__/with-cache/package.json index a41310c..bcd0646 100644 --- a/packages/utils-test/__fixtures__/with-cache/package.json +++ b/packages/utils-test/__fixtures__/with-cache/package.json @@ -3,6 +3,6 @@ "license": "MIT", "version": "0.1.0", "scripts": { - "compile": "copy src/index.ts lib/index.js" + "compile": "node node_modules/.bin/copy src/index.ts lib/index.js" } } diff --git a/yarn.lock b/yarn.lock index ac0487b..5951ed6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1326,6 +1326,11 @@ resolved "https://registry.yarnpkg.com/@types/normalize-package-data/-/normalize-package-data-2.4.0.tgz#e486d0d97396d79beedd0a6e33f4534ff6b4973e" integrity sha512-f5j5b/Gf71L+dbqxIpQ4Z2WlmI/mPJ0fOkGGmFgtb6sAu97EPczzbS3/tJKxmcYDj55OX6ssqwDAWOHIYDRDGA== +"@types/normalize-path@^3.0.0": + version "3.0.0" + resolved "https://registry.yarnpkg.com/@types/normalize-path/-/normalize-path-3.0.0.tgz#bb5c46cab77b93350b4cf8d7ff1153f47189ae31" + integrity sha512-Nd8y/5t/7CRakPYiyPzr/IAfYusy1FkcZYFEAcoMZkwpJv2n4Wm+olW+e7xBdHEXhOnWdG9ddbar0gqZWS4x5Q== + "@types/stack-utils@^1.0.1": version "1.0.1" resolved "https://registry.yarnpkg.com/@types/stack-utils/-/stack-utils-1.0.1.tgz#0a851d3bd96498fa25c33ab7278ed3bd65f06c3e"