-
-
Notifications
You must be signed in to change notification settings - Fork 515
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
Alert disappears instantly leads to org.openqa.selenium.NoAlertPresentException #2470
Comments
Dig into the code but I don't think Serenity (or Selenium, for that matter) does anything special to clear an alert dialog automatically - it looks like maybe a chrome configuration thing. |
chrome works fine, if i click the link by hand all works good if the click happens via serenity id disappears instantly so i dont think its an chrome issue. |
Possible a chrome driver issue. Have you tried with other browsers? |
Otherwise, dig into the code and see if there is a point where something the code is doing makes the alert disappear, or if it happens when the WebDriver call is made. |
Please see attached video, when clicking the link (several times) with serenity its totally not working. Screen.Recording.2021-05-27.at.10.27.07.mov |
There may well be an issue, but the question is where it is. I've never seen this behaviour before so can't say. It could be something the Serenity action is doing, something the Selenium code is doing, or something the chrome driver is doing. Simulating it by hand doesn't help isolate the problem - you'll need to step into the code with a debugger and find at what point the alert is being closed. |
all right thanks |
The problem comes from chromedriver. Now alert will be dismissed is default. If you want to handle the alert pop-up, it should be overrided the capability Try with this. Put in in the serenity.properties or serenity.conf
|
@punkratz312 Pls check and close issue |
great thank you 🙏
…Sent from my iPhone
On 10. Jun 2021, at 11:25, Jacob Vu ***@***.***> wrote:
The problem comes from chromedriver. Now alert will be dismissed is default. If you want to handle the alert pop-up, it should be overried the capability
Try with this. Put in in the serenity.properties or serenity.conf
driver_capabilities.common.unexpectedAlertBehaviour=ignore
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
great. thank you! saved my time a lot. |
Reproduced in Serenity 3.3.1 and 3.3.2 with chromedriver 97.0.4692.71. Added the setting from above to my The alert is closed in the following call stack:
So far, so good. We violated a precondition by not "expecting" the alert. That would be a user error, no ill behaviour has happened so far. But from here, sadly, it all starts to go down the drain: In
At this point, we have no alert and no screenshot. The next action (clicking on a button inside the alert) will fail, because the alert no longer exists. We will have no clue why, because manually doing the same steps will work just fine. I am pretty sure this is a bug in Serenity, not in chromedriver: Expected behaviour would be Serenity failing the test case and informing me that I have to expect the alert. ... Actually, it might even be two bugs: If I wanted a screenshot and cannot get one - surely that is an error!? Please advise how to proceed, @Jacobvu84 , @wakaleo . Should I open a new issue? Should this one be re-opened? Can I do more to help? FWIW: I tried to search for "Serenity expect alert" or "Selenium expect alert" to learn how I would go about turning this "unexpected" alert into an expected one - at least in theory, the alert should then no longer be destroyed and my test case should work. (Then I could inspect the working code in order to form an understanding about how the broken code should work.) Unfortunately I could not unearth pertaining results. References welcome! Here is the step definition I used. I am not yet experienced with Serenity, so YMMV, but it looks to me reasonable for a simple start. public interface My {
Duration PATIENCE = Duration.ofSeconds(5l);
}
public class MainPage extends PageObject {
static final Target USER_INFO_BUTTON = /* user menu button on the page */;
public Performable openUserInfo() {
return Task.where("{0} opens the user menu"
, Ensure.that(USER_INFO_BUTTON.waitingForNoMoreThan(My.PATIENCE)).isDisplayed()
, Click.on(USER_INFO_BUTTON)
);
}
}
public class Logout extends PageObject {
static final Target LOGOUT = /* logout button in the user menu, which will display the alert */;
public Performable logout() {
return Task.where("{0} clicks on Logout"
, Ensure.that(LOGOUT.waitingForNoMoreThan(My.PATIENCE)).isDisplayed()
, Click.on(LOGOUT)
);
}
}
public class Alert extends PageObject {
public Performable clickAlertOK() {
return new ClickOnAlert(); // could not yet figure out how to do this with Task.where syntax
}
class ClickOnAlert implements Task {
@Override
public void performAs(final Actor actor) {
final var alert = new WebDriverWait(getDriver(), My.PATIENCE)
.until(ExpectedConditions.alertIsPresent()); // alert already gone => will fail with TimeoutException
// unreachable
alert.accept();
}
}
}
public class LogoutDefinitionSteps {
Alert alert;
Logout logout;
MainPage mainPage;
@Then(/* Cucumber step to logout */)
public void logout(final Actor actor) {
actor.attemptsTo(
this.mainPage.openUserMenu(),
this.logout.logout(),
this.alert.clickAlertOK()
);
}
} |
More FWIW: I have reproduced the same scenario in Testerra 1.2 and that one handles the logout alert correctly, using the same chromedriver. So yes, looks like a pure Serenity issue. |
If you can trace this back to Serenity code, can you dig into the code and propose a fix in a PR? |
Mmh. I fear I'd need some direction, as to how the corrected behaviour should look like:
... I will try to find out first, how to fix the user error(?) that makes this situation arise. To that end I have posted a StackOverflow question. Once I know how the working test case looks like, maybe I can devise proper error handling for the broken one. (By my current understanding these might all be valid approaches, depending on constraints I am not yet aware of:
) |
... I have found this piece of code in // For now, we'll inelegantly special case unhandled alerts.
if ("unexpected alert open".equals(error) &&
HTTP_INTERNAL_ERROR == encodedResponse.getStatus()) {
String text = "";
Object data = obj.get("data");
if (data != null) {
Object rawText = ((Map<?, ?>) data).get("text");
if (rawText instanceof String) {
text = (String) rawText;
}
}
response.setValue(new UnhandledAlertException(message, text));
} and further instances of the behaviour here and here. The latter issue also claims (in the labels) that Firefox is just as affected as Chrome. It would thence seem to me that screenshot taking during visible alerts is an operation generally unsupported by WebDriver. Proposed fix thence:
I do not think we can do better, because at the time we get the WebDriver response in (I guess one could try to implement some sort of automatic retry of the current step? But I do not see this working, because at least on the page I used for testing, one would have to replay the last four steps to recreate the alert (the logout button is inside a menu which closes when the alert is triggered, so we have to first reopen the menu, and only then can replay the step whose call stack we are currently in.) ... Does that sound like a proper solution? Then I'll try to create a PR. - Advice on the API welcome (e.g. how the methods should be called and into which classes they should go). |
We generally don't fail a test if a screenshot cannot be taken, because this in itself doesn't mean that the application logic is broken, just that the framework can't take a screenshot for some reason. The broader problem here is that you don't necessarily know if an alert will be triggered, so a more robust solution would be to avoid taking screenshots at all if an alert is present (presuming this is a Webdriver limitation). I imagine you could do this either in the PhotoSession class in the takeScreenshot() method (by catching the exception), and/or in the shouldTakeScreenshots() method in BaseStepListener (by checking whether an alert is currently being displayed). I've implemented a simple approach in the current main branch (dce6b5b) - see if this approach works for your problem. |
I think that will not work because at the time you have that exception, the alert has already been destroyed and the session is thus corrupted. But I figured: Can we not in one of the "may I take screenshots" methods - e.g. in E.g. change private boolean driverCanTakeScreenshots() {
return (TakesScreenshot.class.isAssignableFrom(getDriverClass()));
} to private boolean driverCanTakeScreenshots() {
final boolean isGenerallyCapable = TakesScreenshot.class.isAssignableFrom(getDriverClass());
if (isGenerallyCapable) {
try { // slow but...
@SuppressWarnings("unused")
final var unused = this.getProxiedDriver().switchTo().alert();
}
catch(final NoAlertPresentException e) { // ... does not destroy alert
return true;
}
}
return false;
} (But probably rather in ... Will test your code and mine and report back. |
I also thought of checking with switchtTo().alert(), which could work. The only issues are that it will slow down the tests (you have to do it for every screenshot), and it could potentially interfere with the tests if they have already switched to other frames or dialogs etc. One option might be to check for the alert just before taking the screenshot itself - screenshots are a slow operation anyway. Do you have a sample project to reproduce this? |
Yupp, tried yours and mine, as expected yours does not change the behaviour while mine does. Re speed: I think in
I still think just failing at the exception and directing the user to use a different API call would be best, but unfortunately that approach seems to be off the table? Created an MVE. |
Could you have a look at the latest code on main? I've integrated your approach. |
The problem with forcing the user to disable screenshots in interactions that trigger alerts is that it could be very hard to know what action might generate an alert, and it would bind the test code very tightly to the UI, which could make the tests fragile. |
It gets worse... just noticed on my MVE. The code I posted up here works afterwards, but there I used ... Ugh. One thing at a time. Will add a "dumbified" test to the MVE for this issue. Will keep the "too smart" test for the next issue (created #2894 ). |
Did you have a look at my latest push? I used Switch.toAlert().text() to avoid interacting with the alert |
Just did. Also works. Given you put it directly on main, I can delete my branch, yes? |
by clicking a link which triggers and alter dialog it instantly disappears and code does break due to NoAlertPresentException.
when clicking the link by hand it works fine, so maybe there is an issue...
working example (with manual clicking workaround)
workaround.mov
failing test due to instant disappear of alert dialog:
failure.mov
Test:
Clicking action: (find correct element but instant disappear)
chrome.switches:
The text was updated successfully, but these errors were encountered: