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

[CRW-5013] Fix up factory E2E test to avoid 'Restricted' mode #22672

Merged
merged 10 commits into from
Nov 16, 2023
Merged

Conversation

@artaleks9 artaleks9 added status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. area/qe labels Nov 15, 2023
@artaleks9 artaleks9 requested review from dmytro-ndp and removed request for dmytro-ndp November 15, 2023 16:50
@che-bot che-bot added kind/bug Outline of a bug - must adhere to the bug report template. target/branch Indicates that a PR will be merged into a branch other than master. labels Nov 15, 2023
@artaleks9 artaleks9 removed the kind/bug Outline of a bug - must adhere to the bug report template. label Nov 15, 2023
* manage to 'Trusted' Workspace Mode, when the trust dialog does not appear
* @param scmProvider SingleScmProvider object
*/
async manageWorkspaceTrust(scmProvider: SingleScmProvider): Promise<void> {
Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO manageWorkspaceTrust method name is too vague.
What about performManageWorkspaceTrustBox() method name?

Also it would be nice to say in method comment that it is the box appeared in Source Control View.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@che-bot che-bot added the kind/bug Outline of a bug - must adhere to the bug report template. label Nov 16, 2023
@@ -131,6 +131,8 @@ suite(
const scmView: NewScmView = new NewScmView();
await driverHelper.waitVisibility(webCheCodeLocators.ScmView.inputField);
[scmProvider, ...rest] = await scmView.getProviders();
await projectAndFileTests.manageWorkspaceTrust(scmProvider);
Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO, we can make the logic of script more obvious and avoid second execution of [scmProvider, ...rest] = await scmView.getProviders() when extract if (scmProvider === undefined) { verification from manageWorkspaceTrust() to test script level.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@dmytro-ndp
Copy link
Contributor

@artaleks9 : if I understood it correctly this script doesn't check Source Control View functionality itself, but making it possible to proceed with the whole test scenario, which looks like a workaround. Am I right?

@artaleks9
Copy link
Contributor Author

artaleks9 commented Nov 16, 2023

@dmytro-ndp
Yes, you're right

Copy link
Contributor

@dmytro-ndp dmytro-ndp left a comment

Choose a reason for hiding this comment

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

Looks good to merge.

@artaleks9 artaleks9 merged commit ddec5bf into main Nov 16, 2023
2 checks passed
@artaleks9 artaleks9 deleted the crw-5013 branch November 16, 2023 19:52
@che-bot che-bot removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/qe kind/bug Outline of a bug - must adhere to the bug report template. target/branch Indicates that a PR will be merged into a branch other than master.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants