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

A workaround for issue #565 #566

Merged
merged 3 commits into from
May 31, 2023
Merged

Conversation

haukex
Copy link
Contributor

@haukex haukex commented May 31, 2023

This is what I was able to come up with to fix #565, that is, it allows the tests to be run multiple times without them leaving processes behind or leaving port 8000 open. However, I'm having trouble keeping track of how the devtools start up and shut down servers, so I'm uncertain if this is a good solution; it may just be a workaround or hack...

This change allows the tests to run multiple times without leaving
processes behind or leaving port 8000 open.
@haukex haukex mentioned this pull request May 31, 2023
4 tasks
haukex and others added 2 commits May 31, 2023 19:54
@codecov
Copy link

codecov bot commented May 31, 2023

Codecov Report

Merging #566 (cd61a82) into master (45d67a6) will increase coverage by 5.43%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #566      +/-   ##
==========================================
+ Coverage   86.33%   91.77%   +5.43%     
==========================================
  Files          12       12              
  Lines         754      754              
  Branches      124      124              
==========================================
+ Hits          651      692      +41     
+ Misses         81       39      -42     
- Partials       22       23       +1     
Flag Coverage Δ
unit 91.37% <ø> (+5.30%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Dreamsorcerer Dreamsorcerer merged commit 696fa2a into aio-libs:master May 31, 2023
@haukex haukex deleted the try_fix_565 branch May 31, 2023 18:07
haukex added a commit to haukex/aiohttp-devtools that referenced this pull request Jun 2, 2023
Dreamsorcerer added a commit that referenced this pull request Jun 28, 2023
* Work around signals on Windows (fixes #548)

As discussed in the issue above, this works around signals not working
properly on Windows by setting up an endpoint that shuts down the
server.

* Update requirements.txt

A few required packages were not named, and updated pytest.

* Fix _stop_dev_server when _session isn't set

* Get tests working

* Added CHANGES file

* Fixups for linter

* Skip a test on Python 3.7

* Delete 548.bugfix

* A workaround for issue #565

This change allows the tests to run multiple times without leaving
processes behind or leaving port 8000 open.

* Revert "Update requirements.txt"

This reverts commit 778ddbe.
This is now taken care of by my pull request #564.

* Apply a suggestion from code review

Co-authored-by: Sam Bull <[email protected]>

* Apply a suggestion from code review

Co-authored-by: Sam Bull <[email protected]>

* The issue #565 workaround needs to be adjusted

Because in this branch we've changed _stop_dev_server to be async

* Added a test for handlers

Tests whether on_startup, cleanup_ctx, and on_shutdown handlers are
called.

* Revert "A workaround for issue #565"

This reverts commit 11eab74.

* Fix #565

Co-authored-by: Sam Bull <[email protected]>

* Remove an unused function

* Turn off fail-fast on tests

* Hopefully make linter happy

* test_runserver_cleanup.py needs the #566 patch

* Copy over a filterwarnings to try and fix failures

* Fix requirements-dev.txt on Python 3.7

* Began work on test_server_cleanup_byurl (unfinished)

* Revert "Began work on test_server_cleanup_byurl (unfinished)"

This reverts commit 54eced8.

* Working shutdown/cleanup code

The manual test in cleanup_app.py passed on Windows 10, Python 3.7 through 3.11.

* The two tests marked non_windows_test now work

* Revert "Turn off fail-fast on tests"

This reverts commit 15317f3.

Not needed anymore now that all tests are passing

* Revert "Fix requirements-dev.txt on Python 3.7"

This reverts commit dcbe025.

PR #569 was rejected and I mainly needed it for testing.

* Make linter happy again

* Update aiohttp_devtools/runserver/watch.py

* Revert af852e0, thereby restoring 54eced8

This reverts commit af852e0,
which reverted commit 54eced8
"Began work on test_server_cleanup_byurl (unfinished)" because
it turns out we can use that code as a basis to test on Linux.

* Added --shutdown-by-url to CLI

* Replaced test_server_cleanup with _byurl version

* Update watch.py

* Update cli.py

* Update config.py

* Update serve.py

* Update cleanup_app.py

* Update test_runserver_cleanup.py

* Update test_runserver_cleanup.py

* Update watch.py

* Update cleanup_app.py

---------

Co-authored-by: Sam Bull <[email protected]>
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.

Tests leave python processes and an open port behind
2 participants