Implement Role constructor and t.useRole (closes #243)
#1315
Conversation
| @@ -4,6 +4,6 @@ | |||
| <title>Cross-domain iframe page</title> | |||
| </head> | |||
| <body> | |||
| <iframe id="iframe" src=http://localhost:3001/legacy-fixtures/native-dialogs-handling/pages/index.html></iframe> | |||
| <iframe id="iframe" src=http://localhost:3001/legacy-fixtures/api/es-next/native-dialogs-handling/pages/index.html></iframe> | |||
There was a problem hiding this comment.
This is a legacy-api functional tests file. I suppose it was changes by a mistake
|
|
||
|
|
||
| describe('Native dialogs handling', function () { | ||
| it('Should remove dialog handler if `null` specified', function () { |
There was a problem hiding this comment.
I think we should add info about this in the documentation topic about setNativeDialogHandler.
It's possible now to pass null as an argument (see https://github.com/DevExpress/testcafe/pull/1315/files#diff-bc2ca31d9db34de5fa61db548150e9aaR192) /cc @VasilyStrelyaev
There was a problem hiding this comment.
Okay, will do this
|
✅ Tests for the commit ddb507d have passed. See details: |
| this.initErr = err; | ||
| } | ||
|
|
||
| if (!this.initErr) { |
There was a problem hiding this comment.
It's can be just after this line
There was a problem hiding this comment.
Yes it can and will make code more compact, but it will ambiguous which code we expect will throw. It's a bad practice to put everything in the try/catch
| this.loginPage = loginPage; | ||
| this.initFn = initFn; | ||
|
|
||
| this.stateSnapshot = null; |
There was a problem hiding this comment.
When I saw it first time I decided that stateSnapshot is some state of role, may be in role context it should call testRunStateSnapshot
There was a problem hiding this comment.
Indeed, it's a state of role which can be applied to test run.
| async restore (callsite) { | ||
| var prevPhase = this.testRun.phase; | ||
|
|
||
| this.testRun.phase = TEST_RUN_PHASE.inBookmarkRestore; |
There was a problem hiding this comment.
I think it better set in testRun
There was a problem hiding this comment.
It's not. How do you imagine that? One-line function that does exactly the same? And it's fine to modify test run phase outside of test run - it's a public property. Moreover, it's already done in fixture-hook-tracker.
| it('Should fail if there error occurred while restoring configuration', function () { | ||
| return runTests('./testcafe-fixtures/errors-test.js', 'Error restoring configuration', { shouldFail: true }) | ||
| .catch(function (errs) { | ||
| expect(errs[0]).contains('- Error while restoring configuration after Role switch - The specified selector does not match any element in the DOM tree.'); |
There was a problem hiding this comment.
We have a special error for cases when iframe does not exist anymore.
May be we should raise it in this case, wht do you think?
There was a problem hiding this comment.
How it can be used? Note that I don't want to introduce any ad-hoc solutions here and just use basic commands machinery.
There was a problem hiding this comment.
You can make it better)
src/errors/test-run/index.js
Outdated
| @@ -411,3 +417,11 @@ export class WindowDimensionsOverflowError extends TestRunErrorBase { | |||
| this.callsite = callsite; | |||
| } | |||
| } | |||
|
|
|||
| export class RoleSwitchInRoleInitializerError extends TestRunErrorBase { | |||
There was a problem hiding this comment.
I think better to move this error (and WindowDimensionsOverflowError too) above this line because another way the located in group of // Native dialog errors
|
✅ Tests for the commit cbbdba6 have passed. See details: |
|
FPR |
src/test-run/bookmark.js
Outdated
| @@ -43,7 +45,12 @@ class TestRunBookmark { | |||
| new SwitchToIframeCommand({ selector: this.iframeSelector }) : | |||
| new SwitchToMainWindowCommand(); | |||
|
|
|||
| await this.testRun.executeCommand(switchWorkingFrameCommand); | |||
| try { | |||
| await this.testRun.executeCommand(switchWorkingFrameCommand); | |||
There was a problem hiding this comment.
Actually a few types of errors can be raised during execution of switching command
|
❌ Tests for the commit c1bf35c have failed. See details: |
|
✅ Tests for the commit c1bf35c have passed. See details: |
|
rebase required |
|
❌ Tests for the commit f478241 have failed. See details: |
|
❌ Tests for the commit 00a1a31 have failed. See details: |
inikulin
changed the title
t.useRole (#243 part1)t.useRole (closes #243)
|
❌ Tests for the commit 00a1a31 have failed. See details: |
1 similar comment
|
❌ Tests for the commit 00a1a31 have failed. See details: |
|
✅ Tests for the commit 00a1a31 have passed. See details: |
|
FPR |
…evExpress#1315) * Implement Role API, refactor server tests * Add Role id * Rename testRun.state to testRun.phase * Allow removal of native dialog handler. Save current iframe selector. * Introduce marker symbol for TestRun. Store current test run speed. * Implement test run bookmark * useRoleMachinery implemented * Implement UseRoleCommand * Implement t.useRole * Functionality implemented. Add basic test. * Test basic functionality * Add error tests * Move body-parser to dev deps * Fix tests * Run particular tests only on desktop browsers * Revert legacy api test-related page * Moved error definitions to the appropriate place * Throw appropriate message on iframe restore * iframe errors * Fix rebase issues
\cc @AlexanderMoskovkin @helen-dikareva @georgiy-abbasov