-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Conversation
* manage to 'Trusted' Workspace Mode, when the trust dialog does not appear | ||
* @param scmProvider SingleScmProvider object | ||
*/ | ||
async manageWorkspaceTrust(scmProvider: SingleScmProvider): Promise<void> { |
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.
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.
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.
Fixed.
@@ -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); |
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.
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.
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.
Fixed
@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? |
@dmytro-ndp |
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.
Looks good to merge.
What does this PR do?
Fixes a problem of failed factory tests due unexpected 'Restricted' Mode
Screenshot/screencast of this PR
Trial launches of
factory-tests
pipeline showed successful completion:-- https://main-jenkins-csb-crwqe.apps.ocp-c1.prod.psi.redhat.com/blue/organizations/jenkins/Testing%2Fe2e%2Fcomplex%2Flatest%2Ffactory%2Ffactory-tests/detail/factory-tests/254/pipeline/208/
-- https://main-jenkins-csb-crwqe.apps.ocp-c1.prod.psi.redhat.com/blue/organizations/jenkins/Testing%2Fe2e%2Fcomplex%2Flatest%2Ffactory%2Ffactory-tests/detail/factory-tests/250/pipeline/133/
What issues does this PR fix or reference?
https://issues.redhat.com/browse/CRW-5013
https://issues.redhat.com/browse/CRW-5295
https://issues.redhat.com/browse/CRW-5297
https://issues.redhat.com/browse/CRW-5298
How to test this PR?
PR Checklist
As the author of this Pull Request I made sure that:
What issues does this PR fix or reference
andHow to test this PR
completedReviewers
Reviewers, please comment how you tested the PR when approving it.