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

Headless Firefox #144

Closed
wants to merge 22 commits into from
Closed

Headless Firefox #144

wants to merge 22 commits into from

Conversation

mpasternak
Copy link
Contributor

@mpasternak mpasternak commented Mar 28, 2020

Hi,

after splinter gets the change from cobrateam/splinter#677 commited, this patch will give you headless Firefox.

@coveralls
Copy link

coveralls commented Mar 28, 2020

Coverage Status

Coverage decreased (-0.09%) to 87.41% when pulling 7084d38 on mpasternak:master into e6af25a on pytest-dev:master.

@bubenkoff
Copy link
Member

@mpasternak as it's merged looks like, will you bump the splinter version up in the requirements?

@mpasternak
Copy link
Contributor Author

@bubenkoff I'd love to, but the version in splinter repo is 0.13.0 (changed 4 months ago) and it is also the latest version on PyPI. I guess the next pytest-splitner version will be 0.13.1 or 0.14.0. Looks like I need to change pytest-splinter requirements to "splinter>0.13.0", is that okay with you? This will effectively block working releases of pytest-splinter until splinter gets the new release done...

@mpasternak
Copy link
Contributor Author

@bubenkoff Hey, how are things. Any progress on this issue? Looks like someone else also submitted a PR about --headless for Firefox...

@bubenkoff
Copy link
Member

sorry for the late response, on your question about splinter>0.13.0 -> definitely a good idea then, please proceed

@mpasternak
Copy link
Contributor Author

Splinter version bumped, please re-review

Copy link
Member

@bubenkoff bubenkoff left a comment

Choose a reason for hiding this comment

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

please add a changelog record and add yourself to authors

Copy link
Member

@bubenkoff bubenkoff left a comment

Choose a reason for hiding this comment

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

please add a changelog record and add yourself to the authors and resolve the conflicts

tony and others added 4 commits September 12, 2020 16:23
* Simplify --splinter-headless

Not entering --splinter-headless infers False (this is backed by
default values in pytest-splinter and probably user intuition).
Entering --headless implies the user wants to run in headless mode.

* Document --splinter-headless behavior

Without argument, behavior is unchanged. `--splinter-headless` without
arguments defaults to 'true'. Optionally false or true can be passed.

* Update authors and changelog
@mpasternak
Copy link
Contributor Author

@bubenkoff Done, thanks

@bubenkoff
Copy link
Member

@mpasternak broken tests?

@bubenkoff
Copy link
Member

@mpasternak also looks like in the PR it displays the changes which are already in master

@mpasternak
Copy link
Contributor Author

What do you mean "PR displays changes which are already in master"?

I remember that I saw some headless Firefox patches merged from #123 , are those this changes you're talking about?

I did --rebase, but I don't have much experience with it. If there's anything that I broke let me know. Should I remove my fork and re-apply those patches?

@bubenkoff
Copy link
Member

if you look here: https://github.com/pytest-dev/pytest-splinter/pull/144/files, you'll see the changes definitely not made by you
so please rebase your branch correctly
https://github.com/edx/edx-platform/wiki/How-to-Rebase-a-Pull-Request

@bubenkoff
Copy link
Member

@mpasternak linters still fail - the easiest fix is just to run black over the code:

black tests pytest_splinter

@mpasternak
Copy link
Contributor Author

TBH this trivial and small change because of my lack of knowledge how rebase works grew up to be a much harder problem.

Would you suggest re-starting this branch again from scratch or is it still salvageable at this point? What's your idea on that?

@zupo
Copy link
Contributor

zupo commented Oct 27, 2020

@mpasternak: if you give me (zupo) push permissions on your fork, I can try doing the rebasing for you.

@bubenkoff
Copy link
Member

@mpasternak tests are failing please check

zupo added a commit to zupo/pytest-splinter that referenced this pull request Nov 3, 2020
@zupo zupo mentioned this pull request Nov 3, 2020
@zupo
Copy link
Contributor

zupo commented Nov 3, 2020

I've reapplied changes proposed in this PR on top of latest master and opened a new PR: #152

@mpasternak
Copy link
Contributor Author

Thanks @zupo, I hope this gets committed

bubenkoff pushed a commit that referenced this pull request Nov 4, 2020
@bubenkoff
Copy link
Member

implemented in 3.3.0

@bubenkoff bubenkoff closed this Nov 4, 2020
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.

None yet

8 participants