-
Notifications
You must be signed in to change notification settings - Fork 83
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
OCC-18: PriceVariantsPart breaks when you have a TextProductAttributeField with unrestricted values #144
Conversation
src/Modules/OrchardCore.Commerce/Drivers/PriceVariantsPartDisplayDriver.cs
Outdated
Show resolved
Hide resolved
src/Modules/OrchardCore.Commerce/Drivers/PriceVariantsPartDisplayDriver.cs
Outdated
Show resolved
Hide resolved
src/Modules/OrchardCore.Commerce/Recipes/OrchardCore.Commerce.Development.Setup.recipe.json
Outdated
Show resolved
Hide resolved
context.Exists(By.Id("mainNav")); | ||
context.Driver.Title.ShouldBe("Orchard Setup"); |
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.
Why is this necessary? I think the context.Exists(By.Id("mainNav"))
is meant to check that the setup is concluded, while Title.ShouldBe("Orchard Setup")
suggests the opposite.
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.
Well it was failing with context.Exists(By.Id("mainNav"))
which I couldn't resolve because it made no sense to me.
I checked the logs and when this SetupHelpers.RunSetupAsnyc() was called from the new UI test the following happened:
ExecuteTestAfterSetupAsync()
called the setup and all default values were presented to the setup page and Finish button was click but as soon as the setup finished the UI test went nuts right into the SignInDirectlyAsync()
and it never reached the HomePage after setup. It wanted to check mainNav existance on the Login Page which ended up failing which is understandable.
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.
which I couldn't resolve because it made no sense to me.
What do you mean? It worked until now and we use this formula in most other projects' UI tests too.
but as soon as the setup finished the UI test went nuts right into the SignInDirectlyAsync()
You shouldn't be redirected to the login page, that's an error that needs to be fixed.
More importantly, as I wrote the current context.Driver.Title.ShouldBe("Orchard Setup");
passes while the setup is still processing. So you no longer guarantee to wait until the setup finished. This will lead to timing errors down the line and it will be a hell to debug then.
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.
After spending hours trying to figure out what could cause this issue I failed.
If I set up the site manually it works perfectly. The WelcomePage appears there is mainNav in the inspection so everything works as they should.
When I am running the UI test however the same thing happens.
These are subsequent screens taken by the UI test. You can see it never reached the Welcome Page but I assume that the Setup has finished correctly because the logs say so:
`2022-08-08 12:37:25.0097 INFO > Click "Finish Setup" button
2022-08-08 12:37:25.0098 TRACE - > Execute behavior ClicksUsingClickMethodAttribute against "Finish Setup" button
2022-08-08 12:37:31.2786 TRACE - < Execute behavior ClicksUsingClickMethodAttribute against "Finish Setup" button (6.268s)
2022-08-08 12:37:31.2787 INFO < Click "Finish Setup" button (6.268s)
2022-08-08 12:37:31.4158 INFO > Exists applied to:
By.Id: mainNav
2022-08-08 12:37:41.4371 INFO < Exists applied to:
By.Id: mainNav (10.021s) >> OpenQA.Selenium.NoSuchElementException: Unable to locate visible element:...
2022-08-08 12:37:41.4469 - An exception has occurred while interacting with the page https://localhost:9095/Login?ReturnUrl=%2F (Orchard Setup).
2022-08-08 12:37:41.5674 - The test failed with the following exception: OpenQA.Selenium.NoSuchElementException: Unable to locate visible element:
- By: id "mainNav"
- Search time: 10.019s
- Search options: {Visibility=Visible, Timeout=5s, RetryInterval=0.5s, IsSafely=False}`
As you can see in the logs after the Finish Setup button is clicked it gets right to the Login page at least that is what I think by the log.
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.
By the way the same substitution is used in SNOW and there also if I swap context.Driver.Title.Shouldbe()
to context.Exist()
SNOW UI tests are starting to fail on unable to locate visible element "navbar"
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.
SNOW is different, because there everything requires authentication. Anonymous access is intentionally disabled. When the setup is finished and you are sent to the home page, you are immediately redirected to the login screen. That's the expected behavior. OCC has no such restriction.
OCC-18
Fixes #106