Closed grandchild closed 1 year ago
(Replacement for accidentally closed #49.)
Thank you! I'm on the road but am excited to review this when I can later this month.
firefox.py
and common.py
test coverage to 100%test_firefox.py
a bit to be more compact and hopefully readable.$ .test/bin/coverage run --source=src -m pytest && .test/bin/coverage report -m
=== test session starts ===
platform linux -- Python 3.11.3, pytest-7.2.0, pluggy-1.2.0
rootdir: /home/me/pycookiecheat
collected 32 items
tests/test_chrome.py ...... [ 18%]
tests/test_common.py ..... [ 34%]
tests/test_firefox.py ..................... [100%]
=== 32 passed, 320 warnings in 14.70s ===
Name Stmts Miss Cover Missing
-------------------------------------------------------------
src/pycookiecheat/__init__.py 6 0 100%
src/pycookiecheat/chrome.py 127 50 61% 47, 88-134, 176-177, 181-209, 219-220, 223, 255, 259, 268, 271, 273, 287-289, 304-305, 334-336
src/pycookiecheat/common.py 21 0 100%
src/pycookiecheat/firefox.py 91 0 100%
-------------------------------------------------------------
TOTAL 245 50 80%
A note on the review:
I'm looking forward to reviewing. Thanks for your contribution and positive attitude!
Also we should sort out the test failures.
For stylistic reasons, would you might grouping your imports?
Didn't I? Can you point me to the place you're thinking of? Or maybe you're thinking of a different grouping? I aimed for the following:
Also we should sort out the test failures.
Oh, yes definitely. The CI doesn't seem to like the HTTP server I'm running on localhost
during the outermost tests. 🤔 Maybe the CI env doesn't have a loopback interface? Will have to check on a separate branch...
(If you run them locally on your system I'd hope they work...)
Can you point me to the place you're thinking of?
Sorry for not being more clear; I meant that my style preference for import grouping is usually from foo import Foo, Bar
as opposed to separate lines:
from foo import Foo
from foo import Bar
(I recognize there are some benefits for the latter esp with diffing, but for now I still prefer the former.)
(If you run them locally on your system I'd hope they work...)
Locally it looks like the tests are working but lints are failing, mostly missing docstrings.
I think the code itself looks well designed and well thought-through. Thank you!
import grouping is usually
from foo import Foo, Bar
as opposed to separate lines
Ah, sure, will do! 🙂
lints are failing, mostly missing docstrings
I must have missed the linting tests, I'll look for them, and add docstrings to methods that don't have them yet 👍🏾
Maybe the CI env doesn't have a loopback interface?
That doesn't seem to be it.
https://github.com/grandchild/pycookiecheat/actions
The other suspicion I had (requiring 127.0.0.1
instead of localhost
as server address) was also falsified. :| I have to think of other things to test. Maybe the server doesn't come up quick enough? I'll dig some more.
🥳 as you can see in the actions link above, waiting for 1000ms instead of 100ms fixed the tests in my CI test branch. So 100ms was a bit optimistic after all!
CI was green on my test branch, let's see... 😄
(Maybe you already know this, but this great github feature is so well-hidden that it bears repeating: The "force-pushed" part oft the update status line above is a link that shows you the diff of what my force-push changed wrt the state of the branch before.)
Excellent, thank you!
Description
Add a new
firefox_cookies()
, similar tochrome_cookies()
. The arguments are slightly different:profile_name
instead ofcookie_file
, since Firefox supports multiple profiles in the default config directory.profile_name
may be a glob pattern because Firefox profiles have a random prefix that's not directly user-visible or -maintained.password
argument, since Firefox doesn't encrypt its cookies.Firefox stores its cookies in an SQLite3 database file. While Firefox is running, the database is locked, so the code makes a temporary copy of it before reading cookies from it.
If no
profile_name
is given the default profile will be returned, by parsing theprofiles.ini
file in the Firefox config directory. This can be overridden by passing a profile name or pattern toprofile_name
.A note on the unit tests: They don't test the scenario where Firefox is running while getting the cookies. This is because the synchronous playwright API doesn't support yielding from inside the running
with
context manager. The asynchronous API probably does, but I haven't looked into using that. Nonetheless the code supports retrieving cookies while Firefox is running.Closes n8henrie/pycookiecheat#48.
Status
IN DEVELOPMENT
The code is ready to use (and review). I will add more commits to this branch deduplicating code with the chrome code, but I wanted the original commit adding Firefox support to just add code.
Related Issues
Todos
Steps to Test or Reproduce
E.g.: