Skip to content

replace use of scripts with entry_points #311

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 14 commits into from
Apr 9, 2022

Conversation

bollwyvl
Copy link
Contributor

@bollwyvl bollwyvl commented Mar 27, 2022

References

Changes

  • moves scripts/gql-cli to a gql_cli function in gql/cli.py
    • actually delete scripts/gql-cli
  • update setup.py
  • update MANIFEST.in
  • update Makefile

@codecov-commenter
Copy link

codecov-commenter commented Mar 27, 2022

Codecov Report

Merging #311 (d380c47) into master (fe213c4) will decrease coverage by 0.70%.
The diff coverage is 25.00%.

@@             Coverage Diff             @@
##            master     #311      +/-   ##
===========================================
- Coverage   100.00%   99.29%   -0.71%     
===========================================
  Files           26       26              
  Lines         2115     2135      +20     
===========================================
+ Hits          2115     2120       +5     
- Misses           0       15      +15     
Impacted Files Coverage Δ
gql/cli.py 91.12% <25.00%> (-8.88%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fe213c4...d380c47. Read the comment docs.

@bollwyvl
Copy link
Contributor Author

I'll work to get it added to the test suite for coverage.

@leszekhanusz
Copy link
Collaborator

Thanks for your PR, that seems like a good idea.

I'll work to get it added to the test suite for coverage.

Great!

@bollwyvl
Copy link
Contributor Author

This is actually quite useful data: windows does not support all of the signal handlers!

 ____________________ test_ep_aiohttp_using_cli[subprocess] ____________________
  
  event_loop = <ProactorEventLoop running=False closed=False debug=False>
  aiohttp_server = <function aiohttp_server_base.<locals>.go at 0x0000021F5AF43C10>
  monkeypatch = <_pytest.monkeypatch.MonkeyPatch object at 0x0000021F5AF1A160>
  script_runner = <ScriptRunner subprocess>
  run_sync_test = <function run_sync_test.<locals>.run_sync_test_inner at 0x0000021F5AEFDF70>
  
      @pytest.mark.asyncio
      @pytest.mark.script_launch_mode("subprocess")
      async def test_ep_aiohttp_using_cli(
          event_loop, aiohttp_server, monkeypatch, script_runner, run_sync_test
      ):
          from aiohttp import web
      
          async def handler(request):
              return web.Response(text=query1_server_answer, content_type="application/json")
      
          app = web.Application()
          app.router.add_route("POST", "/", handler)
          server = await aiohttp_server(app)
      
          url = str(server.make_url("/"))
      
          def test_code():
      
              monkeypatch.setattr("sys.stdin", io.StringIO(query1_str))
      
              ret = script_runner.run(
                  "gql-cli", url, "--verbose", stdin=io.StringIO(query1_str)
              )
      
              assert ret.success
      
              # Check that the result has been printed on stdout
              captured_out = str(ret.stdout).strip()
      
              expected_answer = json.loads(query1_server_answer_data)
              print(f"Captured: {captured_out}")
              received_answer = json.loads(captured_out)
      
              assert received_answer == expected_answer
      
  >       await run_sync_test(event_loop, server, test_code)
  
  tests\test_console_scripts.py:56: 
  _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
  tests\conftest.py:491: in run_sync_test_inner
      await test_task
  c:\hostedtoolcache\windows\python\3.8.10\x64\lib\concurrent\futures\thread.py:57: in run
      result = self.fn(*self.args, **self.kwargs)
  _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
  
      def test_code():
      
          monkeypatch.setattr("sys.stdin", io.StringIO(query1_str))
      
          ret = script_runner.run(
              "gql-cli", url, "--verbose", stdin=io.StringIO(query1_str)
          )
      
  >       assert ret.success
  E       assert False
  E        +  where False = <pytest_console_scripts.RunResult object at 0x0000021F5AF7E880>.success
  
  tests\test_console_scripts.py:45: AssertionError
  ---------------------------- Captured stdout call -----------------------------
  # Running console script: gql-cli http://127.0.0.1:50955/ --verbose
  # Script return code: 1
  # Script stdout:
  
  # Script stderr:
  Traceback (most recent call last):
    File "c:\hostedtoolcache\windows\python\3.8.10\x64\lib\runpy.py", line 194, in _run_module_as_main
      return _run_code(code, main_globals, None,
    File "c:\hostedtoolcache\windows\python\3.8.10\x64\lib\runpy.py", line 87, in _run_code
      exec(code, run_globals)
    File "D:\a\gql\gql\.tox\py38\Scripts\gql-cli.exe\__main__.py", line 7, in <module>
    File "D:\a\gql\gql\gql\cli.py", line 429, in gql_cli
      loop.add_signal_handler(signal, main_task.cancel)
    File "c:\hostedtoolcache\windows\python\3.8.10\x64\lib\asyncio\events.py", line 536, in add_signal_handler
      raise NotImplementedError
  NotImplementedError
  

@leszekhanusz
Copy link
Collaborator

This is actually quite useful data: windows does not support all of the signal handlers!

Oops. I could swear I tested it on Windows... That's indeed a good thing to also test the few lines launching the script...

@bollwyvl
Copy link
Contributor Author

Moving the import got further on the single_extra excursions: tempted to just throw a skip in there, but what's the right way to do it?

@bollwyvl
Copy link
Contributor Author

way to do it?

moved them back into respective files, seems to skip correctly.

@leszekhanusz
Copy link
Collaborator

leszekhanusz commented Mar 27, 2022

moved them back into respective files, seems to skip correctly.

I think it just needed the @pytest.mark.aiohttp mark, (it is set globally at the beginning of the test_aiohttp.py file so that's why it works now)

@bollwyvl
Copy link
Contributor Author

needed the @pytest.mark.aiohttp mark

well, i guess integrated in the existing test cases is probably better, long term, than something special

@leszekhanusz
Copy link
Collaborator

Any idea why none of the lines in the try: block are detected by the coverage while the test should cover those lines? Is there something we could do about that? Probably related to the fact that it is run in a subprocess.

@bollwyvl
Copy link
Contributor Author

hm: this is working locally, and guided implementation. i'm invoking it directly with pytest and not through tox, but the coverage preservation is exactly why i wanted to introduce the new test dependency.

Screenshot from 2022-03-27 17-00-27

@bollwyvl
Copy link
Contributor Author

it looks like codecov hasn't updated, perhaps?

@leszekhanusz
Copy link
Collaborator

I just tried it on my machine and I have the same result than codecov. Running make tests which is equivalent to pytest tests --cov=gql --cov-report=term-missing -vv gives me:

Name                                           Stmts   Miss  Cover   Missing
----------------------------------------------------------------------------
gql/__init__.py                                    4      0   100%
gql/__version__.py                                 1      0   100%
gql/cli.py                                       169     15    91%   419-448
...

What command are you using to show the coverage?

@bollwyvl
Copy link
Contributor Author

  • Here's what i was using:
pytest tests -vv --ff -x --cov gql --cov-report term-missing:skip-covered --cov-report html:build/htmlcov --html build/pytest.html
  • My outer virtual env is set up with conda on python 3.8, maybe it's some version drift on some transient dependency?
pip list
Package                       Version   Editable project location
----------------------------- --------- ---------------------------
aiofiles                      0.8.0
aiohttp                       3.8.1
aiosignal                     1.2.0
alabaster                     0.7.12
appdirs                       1.4.4
async-timeout                 4.0.2
attrs                         21.4.0
Babel                         2.9.1
black                         19.10b0
botocore                      1.24.27
brotlipy                      0.7.0
build                         0.7.0
certifi                       2021.10.8
cffi                          1.15.0
charset-normalizer            2.0.12
check-manifest                0.48
click                         8.0.4
colorama                      0.4.4
commonmark                    0.9.1
coverage                      6.3.2
cryptography                  36.0.2
distlib                       0.3.4
docutils                      0.16
filelock                      3.6.0
flake8                        3.8.1
frozenlist                    1.3.0
future                        0.18.2
gql                           3.1.0     ~/gql
graphql-core                  3.2.0
idna                          3.3
imagesize                     1.3.0
importlib-metadata            4.11.3
iniconfig                     1.1.1
isort                         4.3.21
Jinja2                        3.1.1
jmespath                      1.0.0
MarkupSafe                    2.1.1
mccabe                        0.6.1
mock                          4.0.2
multidict                     6.0.2
mypy                          0.910
mypy-extensions               0.4.3
nest-asyncio                  1.5.4
packaging                     21.3
parse                         1.19.0
pathspec                      0.9.0
pep517                        0.12.0
pip                           22.0.4
platformdirs                  2.5.1
pluggy                        0.13.1
psutil                        5.9.0
py                            1.11.0
pycodestyle                   2.6.0
pycparser                     2.21
pyflakes                      2.2.0
Pygments                      2.11.2
pyOpenSSL                     22.0.0
pyparsing                     3.0.7
PySocks                       1.7.1
pytest                        6.2.5
pytest-asyncio                0.16.0
pytest-console-scripts        1.3.1
pytest-cov                    3.0.0
pytest-html                   3.1.1
pytest-metadata               2.0.0
python-dateutil               2.8.2
pytz                          2022.1
PyYAML                        6.0
regex                         2022.3.15
requests                      2.27.1
requests-toolbelt             0.9.1
setuptools                    61.1.1
six                           1.16.0
snowballstemmer               2.2.0
Sphinx                        3.5.4
sphinx-argparse               0.2.5
sphinx-rtd-theme              0.5.2
sphinxcontrib-applehelp       1.0.2
sphinxcontrib-devhelp         1.0.2
sphinxcontrib-htmlhelp        2.0.0
sphinxcontrib-jsmath          1.0.1
sphinxcontrib-qthelp          1.0.3
sphinxcontrib-serializinghtml 1.1.5
toml                          0.10.2
tomli                         2.0.1
tox                           3.24.5
typed-ast                     1.5.2
types-aiofiles                0.8.5
types-mock                    4.0.12
types-requests                2.27.15
types-urllib3                 1.26.11
typing_extensions             4.1.1
urllib3                       1.26.9
vcrpy                         4.0.2
virtualenv                    20.14.0
websockets                    10.2
wheel                         0.37.1
wrapt                         1.14.0
yarl                          1.7.2
zipp                          3.7.0
conda list
#
# Name                    Version                   Build  Channel
_libgcc_mutex             0.1                 conda_forge  
_openmp_mutex             4.5                       1_gnu  
aiofiles                  0.8.0              pyhd8ed1ab_0  
aiohttp                   3.8.1            py38h497a2fe_0  
aiosignal                 1.2.0              pyhd8ed1ab_0  
alabaster                 0.7.12                     py_0  
appdirs                   1.4.4              pyh9f0ad1d_0  
async-timeout             4.0.2              pyhd8ed1ab_0  
attrs                     21.4.0             pyhd8ed1ab_0  
babel                     2.9.1              pyh44b312d_0  
black                     19.10b0                    py_4  
botocore                  1.24.27            pyhd8ed1ab_0  
brotlipy                  0.7.0           py38h497a2fe_1003  
bzip2                     1.0.8                h7f98852_4  
ca-certificates           2021.10.8            ha878542_0  
certifi                   2021.10.8        py38h578d9bd_1  
cffi                      1.15.0           py38h3931269_0  
charset-normalizer        2.0.12             pyhd8ed1ab_0  
check-manifest            0.48               pyhd8ed1ab_0  
click                     8.0.4            py38h578d9bd_0  
colorama                  0.4.4              pyh9f0ad1d_0  
commonmark                0.9.1                      py_0  
coverage                  6.3.2            py38h0a891b7_1  
cryptography              36.0.2           py38h2b5fc30_0  
distlib                   0.3.4              pyhd8ed1ab_0  
docutils                  0.16             py38h578d9bd_3  
filelock                  3.6.0              pyhd8ed1ab_0  
flake8                    3.8.1              pyh9f0ad1d_0  
frozenlist                1.3.0            py38h497a2fe_0  
future                    0.18.2           py38h578d9bd_4  
gql                       3.1.0                     dev_0    
graphql-core              3.2.0              pyhd8ed1ab_0  
idna                      3.3                pyhd8ed1ab_0  
imagesize                 1.3.0              pyhd8ed1ab_0  
importlib-metadata        4.11.3           py38h578d9bd_0  
iniconfig                 1.1.1              pyh9f0ad1d_0  
isort                     4.3.21           py38h32f6830_1  
jinja2                    3.1.1              pyhd8ed1ab_0  
jmespath                  1.0.0              pyhd8ed1ab_0  
ld_impl_linux-64          2.36.1               hea4e1c9_2  
libffi                    3.4.2                h7f98852_5  
libgcc-ng                 11.2.0              h1d223b6_14  
libgomp                   11.2.0              h1d223b6_14  
libnsl                    2.0.0                h7f98852_0  
libuuid                   2.32.1            h7f98852_1000  
libzlib                   1.2.11            h166bdaf_1014  
markupsafe                2.1.1            py38h0a891b7_0  
mccabe                    0.6.1                      py_1  
mock                      4.0.2            py38h32f6830_1  
multidict                 6.0.2            py38h497a2fe_0  
mypy                      0.910            py38h497a2fe_4  
mypy_extensions           0.4.3            py38h578d9bd_4  
ncurses                   6.3                  h9c3ff4c_0  
nest-asyncio              1.5.4              pyhd8ed1ab_0  
openssl                   1.1.1n               h166bdaf_0  
packaging                 21.3               pyhd8ed1ab_0  
parse                     1.19.0             pyh44b312d_0  
pathspec                  0.9.0              pyhd8ed1ab_0  
pep517                    0.12.0           py38h578d9bd_1  
pip                       22.0.4             pyhd8ed1ab_0  
platformdirs              2.5.1              pyhd8ed1ab_0  
pluggy                    0.13.1           py38h578d9bd_4  
psutil                    5.9.0            py38h497a2fe_0  
py                        1.11.0             pyh6c4a22f_0  
pycodestyle               2.6.0              pyh9f0ad1d_0  
pycparser                 2.21               pyhd8ed1ab_0  
pyflakes                  2.2.0              pyh9f0ad1d_0  
pygments                  2.11.2             pyhd8ed1ab_0  
pyopenssl                 22.0.0             pyhd8ed1ab_0  
pyparsing                 3.0.7              pyhd8ed1ab_0  
pysocks                   1.7.1            py38h578d9bd_4  
pytest                    6.2.5            py38h578d9bd_2  
pytest-asyncio            0.16.0             pyhd8ed1ab_0  
pytest-console-scripts    1.3.1              pyhd8ed1ab_0  
pytest-cov                3.0.0              pyhd8ed1ab_0  
pytest-html               3.1.1              pyhd8ed1ab_0  
pytest-metadata           2.0.0              pyhd8ed1ab_0  
python                    3.8.13          h582c2e5_0_cpython  
python-build              0.7.0              pyhd8ed1ab_0  
python-dateutil           2.8.2              pyhd8ed1ab_0  
python_abi                3.8                      2_cp38  
pytz                      2022.1             pyhd8ed1ab_0  
pyyaml                    6.0              py38h497a2fe_3  
readline                  8.1                  h46c0cb4_0  
regex                     2022.3.15        py38h0a891b7_0  
requests                  2.27.1             pyhd8ed1ab_0  
requests-toolbelt         0.9.1                      py_0  
setuptools                61.1.1           py38h578d9bd_0  
six                       1.16.0             pyh6c4a22f_0  
snowballstemmer           2.2.0              pyhd8ed1ab_0  
sphinx                    3.5.4              pyh44b312d_0  
sphinx-argparse           0.2.5              pyhd8ed1ab_1  
sphinx_rtd_theme          0.5.2              pyhd8ed1ab_1  
sphinxcontrib-applehelp   1.0.2                      py_0  
sphinxcontrib-devhelp     1.0.2                      py_0  
sphinxcontrib-htmlhelp    2.0.0              pyhd8ed1ab_0  
sphinxcontrib-jsmath      1.0.1                      py_0  
sphinxcontrib-qthelp      1.0.3                      py_0  
sphinxcontrib-serializinghtml 1.1.5              pyhd8ed1ab_1  
sqlite                    3.37.1               h4ff8645_0  
tk                        8.6.12               h27826a3_0  
toml                      0.10.2             pyhd8ed1ab_0  
tomli                     2.0.1              pyhd8ed1ab_0  
tox                       3.24.5           py38h578d9bd_0  
typed-ast                 1.5.2            py38h497a2fe_0  
types-aiofiles            0.8.5              pyhd8ed1ab_0  
types-mock                4.0.12             pyhd8ed1ab_0  
types-requests            2.27.15            pyhd8ed1ab_0  
types-urllib3             1.26.11            pyhd8ed1ab_0  
typing-extensions         4.1.1                hd8ed1ab_0  
typing_extensions         4.1.1              pyha770c72_0  
urllib3                   1.26.9             pyhd8ed1ab_0  
vcrpy                     4.0.2                      py_0  
virtualenv                20.14.0          py38h578d9bd_0  
websockets                10.2             py38h0a891b7_0  
wheel                     0.37.1             pyhd8ed1ab_0  
wrapt                     1.14.0           py38h0a891b7_0  
xz                        5.2.5                h516909a_1  
yaml                      0.2.5                h7f98852_2  
yarl                      1.7.2            py38h497a2fe_1  
zipp                      3.7.0              pyhd8ed1ab_1  
zlib                      1.2.11            h166bdaf_1014  
  • Running tox also yields 100% coverage
  • Running make all_tests also yields 100% coverage, though websocket tests fail...

@leszekhanusz
Copy link
Collaborator

I tried with another conda env with your same python version and progressively modified/added my dependencies to match your pip list but I still have the same coverage detected and missing the lines 419-448

I am still on Ubuntu Linux 20.04.3 LTS

@bollwyvl
Copy link
Contributor Author

bollwyvl commented Mar 28, 2022

I am still on Ubuntu Linux 20.04.3 LTS

As am i. Very strange. It might be a a bit before I can look back into this... trying to really run everything in-process might take a lot more mocks, what with sys.exiting, and the limitations on where/when loops can be started stopped... it could be changed to accept a loop, but then there still wouldn't be any tests as-written.

another option might be to skip pytest-cov and use coverage run pytest ... && coverage report, with which it can be a bit easier to ensure that the bookkeeping is done correctly.

@bollwyvl
Copy link
Contributor Author

Looks like linux py38 is reporting what i'm seeing... I'll try adding a console report to the coverage job so we can see a little more...

@bollwyvl
Copy link
Contributor Author

I also note that the coverage job doesn't have wheel installed... this actually can have impacts on how packages work (* cough * #312).

@bollwyvl
Copy link
Contributor Author

The lint issue is likely psf/black#2964

@leszekhanusz
Copy link
Collaborator

The lint issue is likely psf/black#2964

I am going to update black in a separate PR as it changes the formatting a bit and a lot of files are affected.

@bollwyvl
Copy link
Contributor Author

since coverage is working under tox, what about adding a dedicated testenv:coverage so that it's always executed the same way?

@bollwyvl
Copy link
Contributor Author

bollwyvl commented Apr 9, 2022

Looks like it did the thing?

gql/cli.py                                       169      0   100%

@leszekhanusz
Copy link
Collaborator

Yes, it worked! I had the same behavior on my machine with 100% coverage with tox and not with make tests and was able to pinpoint one of the difference being using -e with pip.

I have no idea why that's the case but it works...

Codecov is not updating the comment but sometimes it can take some time.

@bollwyvl
Copy link
Contributor Author

bollwyvl commented Apr 9, 2022

For all the wheel installs: like this package (#312), parse, brought in as a test dependency, does not distribute wheels. Without the package wheel installed, the old-school python setup.py install approach is used. With wheel available install, it first builds the wheel in an isolated environment, and then installs that, without running arbitrary code in the environment-under-test.

These wheels can also be cached more effectively in CI (which is a separate PR, probably).

As my point in doing all of this is getting to a more-cacheable and -portable state of installing gql, it doesn't seem unreasonable... so would recommend keeping this change as-is.

@leszekhanusz
Copy link
Collaborator

As my point in doing all of this is getting to a more-cacheable and -portable state of installing gql, it doesn't seem unreasonable... so would recommend keeping this change as-is.

Alright!

@leszekhanusz leszekhanusz merged commit c202cca into graphql-python:master Apr 9, 2022
@leszekhanusz
Copy link
Collaborator

Thanks again for your inputs.

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.

Replace use of scripts with cross-platform entry_points
3 participants