-
Notifications
You must be signed in to change notification settings - Fork 346
fix(repo): Always load local clerk-js when running integration tests #2025
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
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
fcf54d0
chore(repo): Install http-server
nikosdouvlis 2c3cc56
chore(repo): Wrap treekill with awaitableTreekill
nikosdouvlis 4f27c94
chore(repo): Server locally-built clerk-js files using http-server
nikosdouvlis 8ccef7e
chore(repo): Serve clerk-js from verdaccio on CICD
nikosdouvlis File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,6 @@ | ||
import * as path from 'node:path'; | ||
|
||
import treekill from 'tree-kill'; | ||
|
||
import { createLogger, fs, getPort, run, waitForIdleProcess, waitForServer } from '../scripts'; | ||
import { awaitableTreekill, createLogger, fs, getPort, run, waitForIdleProcess, waitForServer } from '../scripts'; | ||
import type { ApplicationConfig } from './applicationConfig.js'; | ||
import type { EnvironmentConfig } from './environment.js'; | ||
|
||
|
@@ -63,9 +61,10 @@ export const application = (config: ApplicationConfig, appDirPath: string, appDi | |
stderr: opts.detached ? fs.openSync(stderrFilePath, 'a') : undefined, | ||
log: opts.detached ? undefined : log, | ||
}); | ||
// const shouldRetry = () => proc.exitCode !== 0 && proc.exitCode !== null; | ||
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. Nit: Dead comment 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. This should've been a todo - changing it in the new PR |
||
await waitForServer(serverUrl, { log, maxAttempts: Infinity }); | ||
log(`Server started at ${serverUrl}, pid: ${proc.pid}`); | ||
cleanupFns.push(() => treekill(proc.pid, 'SIGKILL')); | ||
cleanupFns.push(() => awaitableTreekill(proc.pid, 'SIGKILL')); | ||
state.serverUrl = serverUrl; | ||
return { port, serverUrl, pid: proc.pid }; | ||
}, | ||
|
@@ -89,7 +88,7 @@ export const application = (config: ApplicationConfig, appDirPath: string, appDi | |
// If this is ever used as a background process, we need to make sure | ||
// it's not using the log function. See the dev() method above | ||
const proc = run(scripts.serve, { cwd: appDirPath, env: { PORT: port.toString() } }); | ||
cleanupFns.push(() => treekill(proc.pid, 'SIGKILL')); | ||
cleanupFns.push(() => awaitableTreekill(proc.pid, 'SIGKILL')); | ||
await waitForIdleProcess(proc); | ||
state.serverUrl = serverUrl; | ||
return { port, serverUrl, pid: proc }; | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
// @ts-ignore | ||
import treekill from 'tree-kill'; | ||
|
||
export const awaitableTreekill = (pid: number, signal: string) => { | ||
return new Promise<void>((resolve, reject) => { | ||
treekill(pid, signal, err => { | ||
if (err) { | ||
reject(err); | ||
} else { | ||
resolve(); | ||
} | ||
}); | ||
}); | ||
}; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
/* eslint-disable turbo/no-undeclared-env-vars */ | ||
|
||
import os from 'node:os'; | ||
import path from 'node:path'; | ||
|
||
import { stateFile } from '../models/stateFile'; | ||
import { awaitableTreekill, fs, waitForServer } from './index'; | ||
import { run } from './run'; | ||
|
||
export const startClerkJsHttpServer = async () => { | ||
if (process.env.E2E_APP_CLERK_JS) { | ||
return; | ||
} | ||
if (!process.env.CI) { | ||
await copyClerkJsToTempDir(); | ||
} | ||
return serveFromTempDir(); | ||
}; | ||
|
||
export const killClerkJsHttpServer = async () => { | ||
const clerkJsHttpServerPid = stateFile.getClerkJsHttpServerPid(); | ||
if (clerkJsHttpServerPid) { | ||
console.log('Killing clerkJsHttpServer', clerkJsHttpServerPid); | ||
await awaitableTreekill(clerkJsHttpServerPid, 'SIGKILL'); | ||
} | ||
}; | ||
|
||
// If we are running the tests locally, then clerk.browser.js should be built already | ||
// so we simply copy it from packages/clerk to the same location as CICD would install it | ||
const copyClerkJsToTempDir = async () => { | ||
const clerkJsTempDir = getClerkJsTempDir(); | ||
await fs.remove(clerkJsTempDir); | ||
await fs.ensureDir(clerkJsTempDir); | ||
const packagesClerkJsDistPath = path.join(process.cwd(), 'packages/clerk-js/dist'); | ||
fs.copySync(packagesClerkJsDistPath, clerkJsTempDir); | ||
}; | ||
|
||
const serveFromTempDir = async () => { | ||
console.log('Serving clerkJs from temp dir'); | ||
const port = 18211; | ||
const serverUrl = `http://localhost:${port}`; | ||
const now = Date.now(); | ||
const TMP_DIR = path.join(process.cwd(), '.temp_integration'); | ||
const stdoutFilePath = path.resolve(TMP_DIR, `clerkJsHttpServer.${now}.log`); | ||
const stderrFilePath = path.resolve(TMP_DIR, `clerkJsHttpServer.${now}.err.log`); | ||
const clerkJsTempDir = getClerkJsTempDir(); | ||
const proc = run(`node_modules/.bin/http-server ${clerkJsTempDir} -d --gzip --cors -a localhost`, { | ||
cwd: process.cwd(), | ||
env: { PORT: port.toString() }, | ||
detached: true, | ||
stdout: fs.openSync(stdoutFilePath, 'a'), | ||
stderr: fs.openSync(stderrFilePath, 'a'), | ||
}); | ||
stateFile.setClerkJsHttpServerPid(proc.pid); | ||
await waitForServer(serverUrl, { log: console.log, maxAttempts: Infinity }); | ||
console.log('clerk.browser.js is being served from', serverUrl); | ||
}; | ||
|
||
// The location where the clerk.browser.js is served from | ||
// For simplicity, on CICD we install `@clerk/clerk-js` on osTemp | ||
// so the actual clerk.browser.file is at osTemp/clerk-js/node_modules/@clerk/clerk-js/dist | ||
// Locally, it's the osTemp/clerk-js/node_modules/@clerk/clerk-js/dist | ||
// You can override it by setting the `E2E_APP_CLERK_JS_DIR` env variable | ||
const getClerkJsTempDir = () => { | ||
const osTempDir = process.env.E2E_APP_CLERK_JS_DIR || os.tmpdir(); | ||
return path.join(osTempDir, ...'clerk-js/node_modules/@clerk/clerk-js/dist'.split('/')); | ||
}; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Nit: Define the env var in the
env
section below that scriptThere 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.
Opening a second PR, thanks @LekoArts that's cleaner