Skip to content
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

feat(ct): support hot module replacement #936

Merged
merged 1 commit into from
Jun 7, 2024

Conversation

DudaGod
Copy link
Member

@DudaGod DudaGod commented May 23, 2024

What is done

Vite which is used inside component tests support hmr (hot module replacement) it means that it can reload page if something changed (for example if user modify test file). So in this PR I implement ability to rerun test runnables if vite reload page with test.

@DudaGod DudaGod force-pushed the HERMIONE-1507.support_hmr branch 2 times, most recently from baf74bb to a2c9c66 Compare May 23, 2024 09:48
@@ -115,7 +115,7 @@
"@types/node": "18.19.3",
"@types/proxyquire": "1.3.28",
"@types/sharp": "0.31.1",
"@types/sinon": "4.3.3",
"@types/sinon": "17.0.1",
Copy link
Member Author

Choose a reason for hiding this comment

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

We already use sinon 17.0.1, so I update types in order to use clock.tickAsync

@@ -63,6 +63,8 @@ export default (browser: Browser): void => {
const replServer = repl.start({ prompt: "> " });
browser.applyState({ onReplMode: true });

runtimeCfg.extend({ replServer });
Copy link
Member Author

Choose a reason for hiding this comment

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

Add replServer to runtime config in order to be able to stop it if test reconnected in repl mode. I do it because after rerun test replServer opens again and this leads to conflict with new repl server

@@ -49,6 +49,11 @@ const getErrors = (errors: ViteError | ViteError[] = []): ViteError[] => {
};

export const getErrorsOnPageLoad = (initError?: Error): ErrorOnPageLoad[] => {
// ignore error because in this case vite runtime error has more details
if (initError && initError.message.includes("Failed to fetch dynamically imported module")) {
Copy link
Member Author

Choose a reason for hiding this comment

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

I found that this error is not very informative

const connectToSocket = (): BrowserViteSocket => {
const socket = io({
auth: {
runUuid: window.__testplane__.runUuid,
type: BROWSER_EVENT_PREFIX,
reconnect: Boolean(sessionStorage.getItem(RECONNECT_KEY)), // TODO: move reconnect field to constant ????
Copy link
Member Author

Choose a reason for hiding this comment

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

I use reconnect flag in session storage in order to be able to understand if vite/user refresh page with test or not.

this._broInitResOnReconnect = errors;
});

if (this._browser.state.onReplMode) {
Copy link
Member Author

Choose a reason for hiding this comment

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

If repl mode is used I need close previous repl server in order to not conflict with new one, which opens on rerun runnables

}, BRO_INIT_INTERVAL_TIMEOUT_ON_RECONNECT).unref();
})
.timeout(
BRO_INIT_TIMEOUT_ON_RECONNECT,
Copy link
Member Author

Choose a reason for hiding this comment

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

Here I wait only 5 seconds because it should works fast. New url is not open and just refreshed in the same browser tab

_getPreparePageActions(browser: Browser, history: BrowserHistory): (() => Promise<void>)[] {
if (this._isReconnected) {
return super._getPreparePageActions(browser, history);
Copy link
Member Author

Choose a reason for hiding this comment

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

Do not open page again and subscribe on events like callConsoleMethod or runBrowserCommand if browser was reconnected

async run({ sessionId, sessionCaps, sessionOpts, state, ExecutionThreadCls = ExecutionThread }) {
const test = this._test;
const testplaneCtx = test.testplaneCtx || {};
async run({ sessionId, sessionCaps, sessionOpts, state }) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Here I just split this method on three: prepareToRun, runRunnables and finishRun. Code inside them remained the same.

}

// TODO: make it protected
async prepareToRun({ sessionId, sessionCaps, sessionOpts, state }) {
Copy link
Member Author

Choose a reason for hiding this comment

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

I want to use these methods in BrowserTestRunner and stub them. So I want make these methods protected but it is on javascript. Therefore, I left todo here for now

socketClientStub.returns(socket);

const promise = run_();
await clock.tickAsync(1);
Copy link
Member Author

Choose a reason for hiding this comment

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

used in order to give event loop ability to run source code (subscribe on initialize event)

@DudaGod DudaGod force-pushed the HERMIONE-1507.support_hmr branch from a2c9c66 to 059eb55 Compare May 23, 2024 11:24
Copy link
Member

@KuznetsovRoman KuznetsovRoman left a comment

Choose a reason for hiding this comment

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

/ok

src/errors/abort-error.ts Outdated Show resolved Hide resolved
src/errors/abort-error.ts Outdated Show resolved Hide resolved
src/errors/internal-error.ts Outdated Show resolved Hide resolved
src/errors/internal-error.ts Outdated Show resolved Hide resolved
src/worker/browser-env/runner/test-runner/index.ts Outdated Show resolved Hide resolved
src/worker/browser-env/runner/test-runner/index.ts Outdated Show resolved Hide resolved
src/worker/runner/test-runner/index.js Outdated Show resolved Hide resolved
@DudaGod DudaGod force-pushed the HERMIONE-1507.support_hmr branch from 059eb55 to 7bbf0f4 Compare June 7, 2024 16:12
@DudaGod DudaGod merged commit 71628ce into master Jun 7, 2024
2 checks passed
@DudaGod DudaGod deleted the HERMIONE-1507.support_hmr branch June 7, 2024 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants