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

Workaround for modal not appearing during spawn #549

Merged
merged 2 commits into from
Sep 8, 2022

Conversation

lugi0
Copy link
Contributor

@lugi0 lugi0 commented Sep 6, 2022

Signed-off-by: Luca Giorgi lgiorgi@redhat.com

Signed-off-by: Luca Giorgi <lgiorgi@redhat.com>
@lugi0 lugi0 added enhancements Bugfixes, enhancements, refactoring, ... in tests or libraries (PR will be listed in release-notes) do not merge Do not merge this yet please labels Sep 6, 2022
bdattoma
bdattoma previously approved these changes Sep 7, 2022
@lugi0 lugi0 removed the do not merge Do not merge this yet please label Sep 8, 2022
@jgarciao jgarciao self-requested a review September 8, 2022 09:42
jgarciao
jgarciao previously approved these changes Sep 8, 2022
Copy link
Contributor

@jgarciao jgarciao left a comment

Choose a reason for hiding this comment

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

lgtm

Signed-off-by: Luca Giorgi <lgiorgi@redhat.com>
@lugi0 lugi0 dismissed stale reviews from jgarciao and bdattoma via 16c77a6 September 8, 2022 10:00
@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 8, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@lugi0 lugi0 merged commit bf6c93e into red-hat-data-services:master Sep 8, 2022
Comment on lines +138 to +139
IF ${modal}==False
Log message=Starting server modal didn't appear after 60s level=ERROR
Copy link
Contributor

Choose a reason for hiding this comment

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

@lugi0 @jgarciao @bdattoma, the error message shouldn't be after 60s, it should be after 5s, 10s max.

It's fine for the test to wait longer and do what's necessary not to crash when the popup doesn't appear this fast, but you can't have only green lights when this happens...

Copy link
Contributor

Choose a reason for hiding this comment

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

this line would raise a red light since the level is set to ERROR. @lugi0 right?

Copy link
Contributor

Choose a reason for hiding this comment

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

it will raise the red flag ... but only after 60s. The popup should appear "immediately".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancements Bugfixes, enhancements, refactoring, ... in tests or libraries (PR will be listed in release-notes)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants