Skip to content

test: clear the session token before login #8289

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.

Sign up for GitHub

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

Jump to bottom
Merged
openshift-merge-robot merged 1 commit into from
Mar 5, 2021
Merged

test: clear the session token before login #8289

openshift-merge-robot merged 1 commit into from
Mar 5, 2021

Conversation

@spadgett
Copy link
Member

@spadgett spadgett commented Mar 3, 2021 •
edited
Loading

Sometimes Cypress will not correctly clear the session cookie on logout
when running in CI, possibly due to a Cypress bug in cookie handling. We
have not been able to reproduce manually or running the tests outside of
CI. Work around the problem by clearing the cookie manually before login.

Enable cookie debugging in Cypress to help troubleshoot similar problems
in the future.

This also changes the login command to use window.SERVER_FLAGS.authDisabled
to detect a local development environment regardless of whether
BRIDGE_KUBEADMIN_PASSWORD is set.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 3, 2021
@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 3, 2021
@spadgett
Copy link
Member Author

spadgett commented Mar 3, 2021

/cc @dtaylor113 @TheRealJon @zherman0

dtaylor113
dtaylor113 approved these changes Mar 3, 2021
View reviewed changes
Copy link
Contributor

@dtaylor113 dtaylor113 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

/lgtm

@spadgett spadgett changed the title [WIP] test: detect if the user is already logged in test: detect if the user is already logged in Mar 3, 2021
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 3, 2021
@spadgett spadgett force-pushed the cypress-detect-login branch from 5b1ec4a to e4a4093 Compare March 3, 2021 16:06
@dtaylor113
Copy link
Contributor

dtaylor113 commented Mar 3, 2021

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 3, 2021
This was referenced Mar 3, 2021
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
@spadgett
Copy link
Member Author

spadgett commented Mar 3, 2021

Quota exceeded

/retest

@spadgett
Copy link
Member Author

spadgett commented Mar 3, 2021

/retest

TheRealJon
TheRealJon approved these changes Mar 3, 2021
View reviewed changes
Copy link
Member

@TheRealJon TheRealJon left a comment

Choose a reason for hiding this comment

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

/lgtm

@spadgett
Copy link
Member Author

spadgett commented Mar 3, 2021

Unfortunately, even with this change we're still randomly getting 401 errors when viewing a project's details, which kicks us to the login page. Here's a video of it happening.

@TheRealJon
Copy link
Member

TheRealJon commented Mar 3, 2021

Hmm interesting. Some kind of delay for the last logout to take effect?

@spadgett spadgett force-pushed the cypress-detect-login branch 2 times, most recently from c04b6db to f59ed29 Compare March 4, 2021 22:23
@spadgett
Copy link
Member Author

spadgett commented Mar 4, 2021

/retest

@spadgett spadgett force-pushed the cypress-detect-login branch 2 times, most recently from 0b408ce to a44c1d7 Compare March 5, 2021 12:44
@spadgett
test: clear the session token before login
8aa3db6 
Sometimes Cypress will not correctly clear the session cookie on logout
when running in CI, possibly due to a Cypress bug in cookie handling. We
have not been able to reproduce manually or running the tests outside of
CI. Work around the problem by clearing the cookie manually before login.

Enable cookie debugging in Cypress to help troubleshoot similar problems
in the future.

This also changes the login command to use `window.SERVER_FLAGS.authDisabled`
to detect a local development environment regardless of whether
`BRIDGE_KUBEADMIN_PASSWORD` is set.
@spadgett spadgett force-pushed the cypress-detect-login branch from a44c1d7 to 8aa3db6 Compare March 5, 2021 14:33
@spadgett spadgett mentioned this pull request Mar 5, 2021
Closed
@dtaylor113
Copy link
Contributor

dtaylor113 commented Mar 5, 2021

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 5, 2021
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Mar 5, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dtaylor113, spadgett, TheRealJon, zherman0

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@spadgett
Copy link
Member Author

spadgett commented Mar 5, 2021

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 5, 2021
@spadgett
Copy link
Member Author

spadgett commented Mar 5, 2021

/retest

@spadgett
Copy link
Member Author

spadgett commented Mar 5, 2021

/retest

error An unexpected error occurred: "https://registry.yarnpkg.com/uuid/-/uuid-2.0.3.tgz: Request failed \"500 Internal Server Error\"".

@openshift-bot
Copy link
Contributor

openshift-bot commented Mar 5, 2021

/retest

Please review the full test history for this PR and help us cut down flakes.

@spadgett
Copy link
Member Author

spadgett commented Mar 5, 2021

/retest

@openshift-bot
Copy link
Contributor

openshift-bot commented Mar 5, 2021

/retest

Please review the full test history for this PR and help us cut down flakes.

@spadgett spadgett closed this Mar 5, 2021
@spadgett spadgett reopened this Mar 5, 2021
@openshift-bot
Copy link
Contributor

openshift-bot commented Mar 5, 2021

/retest

Please review the full test history for this PR and help us cut down flakes.

@spadgett
Copy link
Member Author

spadgett commented Mar 5, 2021

/retest

@spadgett spadgett mentioned this pull request Mar 5, 2021
Merged
@spadgett
Copy link
Member Author

spadgett commented Mar 5, 2021

/retest

@openshift-merge-robot openshift-merge-robot merged commit 8df24db into openshift:master Mar 5, 2021
@spadgett spadgett deleted the cypress-detect-login branch March 5, 2021 17:47
@benluddy benluddy mentioned this pull request Mar 5, 2021
Merged
@dtaylor113 dtaylor113 mentioned this pull request Mar 9, 2021
Merged
@spadgett spadgett added this to the v4.8 milestone Mar 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TheRealJon TheRealJon TheRealJon approved these changes

+2 more reviewers

@dtaylor113 dtaylor113 dtaylor113 approved these changes

@zherman0 zherman0 zherman0 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@dtaylor113 dtaylor113

@TheRealJon TheRealJon

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Milestone

v4.8

Development

Successfully merging this pull request may close these issues.

7 participants

@spadgett @dtaylor113 @TheRealJon @openshift-bot @openshift-ci-robot @zherman0 @openshift-merge-robot