Skip to content

Commit

Permalink
initial (#4883)
Browse files Browse the repository at this point in the history
  • Loading branch information
miherlosev committed Mar 26, 2020
1 parent 0a619fa commit 36337d3
Show file tree
Hide file tree
Showing 8 changed files with 97 additions and 18 deletions.
18 changes: 12 additions & 6 deletions src/client/driver/driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,10 @@ export default class Driver extends serviceUtils.EventEmitter {
return browser.setActiveWindowId(this.browserActiveWindowId, hammerhead.createNativeXHR, this.windowId);
})
.then(() => {
this._startInternal({ finalizePendingCommand: true });
this._startInternal({
finalizePendingCommand: true,
isFirstRequestAfterWindowSwitching: true
});

this.setAsMasterInProgress = false;
})
Expand Down Expand Up @@ -1017,12 +1020,12 @@ export default class Driver extends serviceUtils.EventEmitter {
};
}

_startInternal (opts = { finalizePendingCommand: false }) {
_startInternal (opts) {
this.role = DriverRole.master;

browser.startHeartbeat(this.heartbeatUrl, hammerhead.createNativeXHR);
this._setupAssertionRetryIndication();
this._startCommandsProcessing(opts.finalizePendingCommand);
this._startCommandsProcessing(opts);
}

_stopInternal () {
Expand All @@ -1049,7 +1052,7 @@ export default class Driver extends serviceUtils.EventEmitter {

}

_startCommandsProcessing (finalizePendingCommand) {
_startCommandsProcessing (opts = { finalizePendingCommand: false, isFirstRequestAfterWindowSwitching: false }) {
const pendingStatus = this.contextStorage.getItem(PENDING_STATUS);

if (pendingStatus)
Expand All @@ -1069,9 +1072,12 @@ export default class Driver extends serviceUtils.EventEmitter {
if (this._failIfClientCodeExecutionIsInterrupted())
return;

finalizePendingCommand = finalizePendingCommand || this._hasPendingActionFlags(this.contextStorage);
const finalizePendingCommand = opts.finalizePendingCommand || this._hasPendingActionFlags(this.contextStorage);

const status = pendingStatus || new DriverStatus({ isCommandResult: finalizePendingCommand });
const status = pendingStatus || new DriverStatus({
isCommandResult: finalizePendingCommand,
isFirstRequestAfterWindowSwitching: opts.isFirstRequestAfterWindowSwitching
});

this.contextStorage.setItem(this.COMMAND_EXECUTING_FLAG, false);
this.contextStorage.setItem(this.EXECUTING_IN_IFRAME_FLAG, false);
Expand Down
20 changes: 11 additions & 9 deletions src/client/driver/status.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@ export default class DriverStatus extends Assignable {
constructor (obj) {
super(obj);

this.id = generateId();
this.isCommandResult = false;
this.executionError = null;
this.pageError = null;
this.resent = false;
this.result = null;
this.consoleMessages = null;
this.isPendingWindowSwitching = false;
this.id = generateId();
this.isCommandResult = false;
this.executionError = null;
this.pageError = null;
this.resent = false;
this.result = null;
this.consoleMessages = null;
this.isPendingWindowSwitching = false;
this.isFirstRequestAfterWindowSwitching = false;

this._assignFrom(obj, true);
}
Expand All @@ -25,7 +26,8 @@ export default class DriverStatus extends Assignable {
{ name: 'pageError' },
{ name: 'result' },
{ name: 'consoleMessages' },
{ name: 'isPendingWindowSwitching' }
{ name: 'isPendingWindowSwitching' },
{ name: 'isFirstRequestAfterWindowSwitching' }
];
}
}
14 changes: 11 additions & 3 deletions src/test-run/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import { UNSTABLE_NETWORK_MODE_HEADER } from '../browser/connection/unstable-net
import WarningLog from '../notifications/warning-log';
import WARNING_MESSAGE from '../notifications/warning-message';
import { StateSnapshot, SPECIAL_ERROR_PAGE } from 'testcafe-hammerhead';
import { NavigateToCommand } from './commands/actions';
import * as INJECTABLES from '../assets/injectables';
import { findProblematicScripts } from '../custom-client-scripts/utils';
import getCustomClientScriptUrl from '../custom-client-scripts/get-url';
Expand Down Expand Up @@ -53,6 +52,7 @@ const AssertionExecutor = lazyRequire('../assertions/executor');
const actionCommands = lazyRequire('./commands/actions');
const browserManipulationCommands = lazyRequire('./commands/browser-manipulation');
const serviceCommands = lazyRequire('./commands/service');
const observationCommands = lazyRequire('./commands/observation');

const { executeJsExpression, executeAsyncJsExpression } = lazyRequire('./execute-js-expression');

Expand Down Expand Up @@ -491,13 +491,21 @@ export default class TestRun extends AsyncEventEmitter {
}

// Handle driver request
_shouldResolveCurrentDriverTask (driverStatus) {
const isFirstExecuteSelectorCommandAfterWindowSwitching =
driverStatus.isFirstRequestAfterWindowSwitching &&
this.currentDriverTask.command instanceof observationCommands.ExecuteSelectorCommand;

return !isFirstExecuteSelectorCommandAfterWindowSwitching;
}

_fulfillCurrentDriverTask (driverStatus) {
if (!this.currentDriverTask)
return;

if (driverStatus.executionError)
this._rejectCurrentDriverTask(driverStatus.executionError);
else
else if (this._shouldResolveCurrentDriverTask(driverStatus))
this._resolveCurrentDriverTask(driverStatus.result);
}

Expand Down Expand Up @@ -788,7 +796,7 @@ export default class TestRun extends AsyncEventEmitter {
}

async navigateToUrl (url, forceReload, stateSnapshot) {
const navigateCommand = new NavigateToCommand({ url, forceReload, stateSnapshot });
const navigateCommand = new actionCommands.NavigateToCommand({ url, forceReload, stateSnapshot });

await this.executeCommand(navigateCommand);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<title>i4855 - Child page</title>
</head>
<body>
<a href="http://localhost:3001/i4855">Open a self-closed page</a>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<title>i4855 - Index page</title>
</head>
<body>
<!--
// NOTE: The problem is reproduced only for the following unstable scenario:
// - 'ExecuteSelectorCommand' is queried on the child page.
// - the child page is closed. At that moment, the result of the 'ExecuteSelectorCommand' was not sent to the server.
// - the parent page became a 'master' page and starts processing commands.
// - the initial command processing request on the new 'master' is interpreted as the 'ExecuteSelectorCommand'.
// This causes the crash down in the 'replicator' module.
// To create a stable reproducible example I am forced to guarantee that the result of the 'ExecuteSelectorCommand' isn't sent to the server.
-->
<a target="_blank" href="child.html">Open a new window</a><br/>
<span>Checked text</span>
</body>
</html>
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,8 @@ describe('Allow multiple windows', () => {
return testCafe.close();
});
});

it("Should not finalize the 'ExecuteSelectorCommand' command on driver starting (GH-4855)", () => {
return runTests('testcafe-fixtures/i4855.js', null, { allowMultipleWindows: true });
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { Selector } from 'testcafe';

fixture `Should not finalize the 'ExecuteSelectorCommand' command on driver starting (GH-4855)`
.page('http://localhost:3000/fixtures/run-options/allow-multiple-windows/pages/i4855/index.html');

test('test', async t => {
await t
.click('a')
.click('a')
.expect(Selector('span').textContent).eql('Checked text');
});
18 changes: 18 additions & 0 deletions test/functional/site/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,24 @@ Server.prototype._setupRoutes = function () {
res.end(parsedUA.name);
});

this.app.get('/i4855', (req, res) => {
res.send(`
<html>
<body>
<script>
var driver = window['%testCafeDriverInstance%'];
driver._onExecuteSelectorCommand = function () {
window.setTimeout(() =>{
window.close();
}, 1000);
};
</script>
</body>
</html>
`);
});

this.app.get('*', function (req, res) {
const reqPath = req.params[0] || '';
const resourcePath = path.join(server.basePath, reqPath);
Expand Down

0 comments on commit 36337d3

Please sign in to comment.