-
Notifications
You must be signed in to change notification settings - Fork 531
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
Experiment/selenium logs #1412
Experiment/selenium logs #1412
Conversation
cc @michaelneale an experiment to use Chrome instead of Firefox so we can view the browser logs from ATH. Doesn't appear to be coming to Firefox any time soon per SeleniumHQ/selenium#1161 and mozilla/geckodriver#284 |
@michaelneale this ran once, successfully, but fails intermittently with "no such container" errors. Any idea why? Works fine in my local environment but I don't fully grasp how we have Docker configured in CI. |
@cliffmeyers running it again. A glitch in the matrix. |
I see this in logs when this fails:
when pulling the image, so it is something about that new image that won't work in some environments, but it seems to work on the bare-docker instance.. just a bad image (possibly, but probably something environmental). It seems a bit change to switch to chrome, is tha tthe only way? given it works on the "bare docker" instance I think I might ask yoann to rework the agents to work that way... |
@cliffmeyers so it will run when it is on a certain agent, but not the bulk. I have asked @ydubreuil to see if we can migrate to the "bare docker" approach, but if there is a way to can dance around this for now... would be good (not sure when he will get to it). |
…ing overall event if object structure is unexpected
…rg name - this was broken after GitHub OrgFolder - MBP change
…aved due to a setValue timing issue that shows up in Chrome
Still working a few things on this one...
|
…to happen first; using "waitForLocationContains" seems more robust
|
||
public class AthModule extends AbstractModule { | ||
@Override | ||
protected void configure() { | ||
|
||
DesiredCapabilities capability = DesiredCapabilities.firefox(); | ||
DesiredCapabilities capability = DesiredCapabilities.chrome(); |
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.
We should really pass this in from tests so we can run on multiple browsers at some point
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.
We also need to take a look at selenium-grid as it's the "selenium way" of firing off multiple test runs in different browsers.
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.
LGTM 🔥
@cliffmeyers I get errors when I run tests locally individually, eg Are you able to run tests locally? one at a time or all together? I am worried about the supportability of this locally if I am seeing errors like this (on master I can run it fine) 🐛
so not sure if this is a 🐛 - but we have to be able to run it locally, so not sure what this is about or just my environment. Can we trust chrome to not update itself inside the image as it launches? It seems that I see different things to what CI does @kzantow's suggestion is to have a flag to choose browsers (but not sure how much of the code change here could apply to ffx) |
perhaps @vivek could try this locally - if testEditor works for him from IntelliJ - it might just be my environment. |
need to confirm this runs locally for people
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.
need to confirm it can run discrete tests locally - which doesn't seem to work for me
Tested it on my local machine and it works for me, I was able to run individual tests from IntelliJ |
@michaelneale yes, I've been running these repeatedly locally from IntelliJ and from command line. Are you able to run Java and nightwatch tests from command line using |
These tests are still a bit flaky apparently. I will put a little more time in to see if I can tighten then up before we consider a merge to master. |
@cliffmeyers I tried nightwatch using the nightwatch CLI - it seemed ok in parts. Running from IntelliJ would fail at some point but started ok... can show you if you like. |
ok looks like an interesting single failure (just testEditor). note the funky screenshot: https://ci.blueocean.io/job/blueocean/job/experiment%252Fselenium-logs/24/artifact/acceptance-tests/target/screenshots/io.blueocean.ath.live.GithubEditorTest_testEditor.png getting close @cliffmeyers this is awesome! |
…le, then click. if we can repro the "can't click on element" error, then this is a good place for retry logic
…t will be a more reliable click target for Selenium
@michaelneale that last screenshot is interesting, I haven't seen that one before. Haven't been able to repro it but I think I have made some progress on this overall. I was finally able to repro an error we see pretty frequently: "selector is not clickable at point (x, y)". This happens when you try to click an item that's being animated or lives in an animating container. Unfortunately even adding I found a fix that appears 100% reliable but I think we should actually use a two-phase approach to handle this:
I think the correct answer is "both" because I am not sure we can write cross-cutting logic that handles every case, and I don't see anything bad about retrying a click since this impl logs very explicitly what it's doing:
If we are in agreement, I'd like to land this in master, let it soak for a few days, then do a separate PR where I go through all Java ATH and change everything to use |
Just to add a little more background: after writing |
@cliffmeyers still not sure - I can't run testEditor locally at all - fails immediately when trying to click. Also might want to update this to master as a few things have gone in that may help... I may have to show you next week. It is most odd. Can @vivek run testEditor locally? it is ok if it is just me, but if I can't help people debug ATH locally, that is a problem. All I did was a clean build - ./run.sh --dev and then run
is there it fails. There are no errors in the chrome, and it is an instant failure, no timeout or anything.
|
hrm... I just saw this on another branch, so perhaps it literally is just my machine being annoying @cliffmeyers ;) |
@cliffmeyers ok I was able to see it pass. It seems something related to timing. I know that chrome on my machine really slams the CPU a lot, maybe that is part of it? So I guess if it is just me, this is looking pretty good ... we are expecting this to be more reliable? |
# Conflicts: # acceptance-tests/src/test/js/failingStages.js
@michaelneale although we discussed your questions offline, will summarize here for posterity: overall I am not sure there is a big increase in stability but it should not be less. I fixed two issues in
|
Description
Submitter checklist
Reviewer checklist