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

Travis - run tests in Firefox #5831

Merged
merged 19 commits into from
Jun 25, 2018
Merged

Travis - run tests in Firefox #5831

merged 19 commits into from
Jun 25, 2018

Conversation

cherniavskii
Copy link
Collaborator

@cherniavskii cherniavskii commented Oct 8, 2017

It is possible to run karma tests in Firefox with a virtual screen.
Do not merge before all errors in FF are fixed.

Tests which should be fixed:

@cherniavskii cherniavskii changed the title Travis run tests in Firefox Travis - run tests in Firefox Oct 8, 2017
@cherniavskii cherniavskii changed the title Travis - run tests in Firefox Travis - run tests in real browsers Oct 10, 2017
@cherniavskii
Copy link
Collaborator Author

Added also Chrome support

@mourner
Copy link
Member

mourner commented Oct 11, 2017

Looks like we'll only be able to turn this on once we sort out the failures on both browsers.

@cherniavskii
Copy link
Collaborator Author

@mourner yes

@cherniavskii cherniavskii changed the title Travis - run tests in real browsers Travis - run tests in Firefox Oct 15, 2017
@cherniavskii
Copy link
Collaborator Author

Moved Chrome support to separate PR #5845

Copy link
Collaborator Author

@cherniavskii cherniavskii left a comment

Choose a reason for hiding this comment

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

Thanks @ghybs for fixing failing tests!

I think we are ready to merge this! 🎉

@cherniavskii cherniavskii requested a review from ghybs June 25, 2018 09:20
@ghybs
Copy link
Collaborator

ghybs commented Jun 25, 2018

I am glad you could solve some test issues by simply specifying the browser width in Travis config!

Now probably we should also improve those tests to positively specify the map container width, so that we do not rely on the browser width. For reference, these tests are:

  • Marker.Drag drag in CSS scaled container drags a marker with mouse, compensating for CSS scale
  • Canvas #events should not fire click when dragging the map on top of it
  • Map.Drag mouse events change the center of the map

Copy link
Collaborator

@ghybs ghybs left a comment

Choose a reason for hiding this comment

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

Nice!

BTW please would you know what exactly the env var DISPLAY=:99.0 is for?

@cherniavskii
Copy link
Collaborator Author

improve those tests to positively specify the map container width

Fair enough, but should it block this PR from merging?

what exactly the env var DISPLAY=:99.0 is for?

Hmm, good question. It is required when using xvbf directly (see https://docs.travis-ci.com/user/gui-and-headless-browsers/).
But since I'm using xvfb-run wrapper, it seems unnecessary now.
I'll try to remove it :)

@cherniavskii
Copy link
Collaborator Author

@ghybs Yep, it works without DISPLAY env variable :) Thanks for your review! :)

@ghybs
Copy link
Collaborator

ghybs commented Jun 25, 2018

should it block this PR from merging?

No, this was mainly for reference for future improvement, sorry I should have made it clearer.

@ghybs ghybs merged commit 9fda888 into master Jun 25, 2018
@ghybs
Copy link
Collaborator

ghybs commented Jun 25, 2018

That is awesome! Thx @cherniavskii!

@cherniavskii
Copy link
Collaborator Author

@ghybs Thanks for your help ! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants