Skip to content

Support maps automation #286

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

Merged
merged 9 commits into from
Nov 19, 2019
Merged

Conversation

Taxanas
Copy link
Contributor

@Taxanas Taxanas commented Nov 7, 2019

PR inclues:

  • 4 new methods to BrowserTest to help automation of maps, dragable graphs and other.
  • Supplement to BrowserTest documentation.
  • Added example to HsacExamples of new methods usage in MapTest.
  • Added acceptance tests with mockups for new methods to HsacAcceptanceTests.

Copy link
Owner

@fhoeben fhoeben left a comment

Choose a reason for hiding this comment

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

Looks quite powerful, but is there any way we could make it work in more 'human' terms instead of pixels?

@Taxanas Taxanas requested a review from fhoeben November 11, 2019 09:42
@Taxanas
Copy link
Contributor Author

Taxanas commented Nov 12, 2019

Hi, @fhoeben ,
Waiting for updates :)
P.s. additionally, when can i expect 4.6.4 release? :D

@Taxanas
Copy link
Contributor Author

Taxanas commented Nov 15, 2019

Hello, @fhoeben ,

I'm not making lots of PR's. Not sure if i'm missing something, that should be done by me, or should i just wait? :D

Thank you,

@fhoeben
Copy link
Owner

fhoeben commented Nov 15, 2019

At this time I don't think I'm waiting on something from you. I just haven't gotten around to looking at your latest changes. Sorry. Can you wait a bit longer?

@Taxanas
Copy link
Contributor Author

Taxanas commented Nov 16, 2019

Sure,
Thank you for response. :)

@fhoeben
Copy link
Owner

fhoeben commented Nov 17, 2019

Have you seen #138? It has the same basic feature you are adding, but I never got around to merging it. Sorry @teunisnl! I have to say I like the way it specifies the offset in the wiki better.

@Taxanas
Copy link
Contributor Author

Taxanas commented Nov 18, 2019

Thank you for detailed review @fhoeben , and my apologies for mistakes i left (after pinging you so hard:) )
I saw mentioned PR, but as there were no activities for 2 years and lack of other offset methods, i made a new one :)

Hope this one aa6590d
Is sufficient, as i made all corrections according your comments.

Hope to hear you soon and have a good day ;)

@Taxanas Taxanas requested a review from fhoeben November 18, 2019 08:19
Copy link
Contributor Author

@Taxanas Taxanas left a comment

Choose a reason for hiding this comment

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

Changes done with commit aa6590d

@fhoeben fhoeben merged commit 43dd6bc into fhoeben:master Nov 19, 2019
@fhoeben
Copy link
Owner

fhoeben commented Nov 19, 2019

For me the test needed a small update. url after zooming is always: https://www.google.com/maps/@51.7147561,11.342505,5.16z

@fhoeben
Copy link
Owner

fhoeben commented Nov 19, 2019

Since that is the same mapUrl used earlier. Does that mean something is broken?
I do see the map change, but the URL does not change...

Screenshot 2019-11-19 at 10 38 20

@Taxanas
Copy link
Contributor Author

Taxanas commented Nov 19, 2019

Hello, @fhoeben

With chromedriver i tried with:
linux [headless chrome], windows [chrome] - autotests succeed.
By hand i tried zooming and checking url changes on:
macOS [chrome], windows [firefox, chrome]... tried with Safari too, but by hand url is shown as locked and im not familiar with macOS or Safari..

There's slight delay for it to change. Thats why i left hardcoded 800 milliseconds 'wait' before checking location.

Perhaps on slower environments this 800 is not enough...

I will think of something

@Taxanas
Copy link
Contributor Author

Taxanas commented Nov 19, 2019

There are 2 options for this issue on public google maps:

  • Increase timeout after which to make check of url changing (lets say to 2 seconds)
  • Clean this check of url changing from map test scope (as following tests would still fail if zooming would fail).

Which one you prefer?

@Taxanas
Copy link
Contributor Author

Taxanas commented Nov 19, 2019

One more idea - i could make example with mock up if you prefer ;)

@fhoeben
Copy link
Owner

fhoeben commented Nov 19, 2019

I don't believe it's a timing issue. Even if I wait 10s (chrome on macOS, fiber internet) the location does not change.

@Taxanas Taxanas mentioned this pull request Nov 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants