Skip to content

Conversation

@elacuesta
Copy link
Member

@elacuesta elacuesta commented Oct 10, 2021

  • Consolidate duplicated initialization code in tests (tests.make_handler)
  • Add a _close coroutine to the main handler (to be awaited directly by the above tests.make_handler, without being wrapped in a Twisted Deferred)
  • Remove closing calls to context objects (they are closed when the browser is closed)

@codecov
Copy link

codecov bot commented Oct 10, 2021

Codecov Report

Merging #32 (ee0f5a6) into master (6a68482) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #32   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            3         3           
  Lines          167       168    +1     
=========================================
+ Hits           167       168    +1     
Impacted Files Coverage Δ
scrapy_playwright/handler.py 100.00% <100.00%> (ø)

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 6a68482...ee0f5a6. Read the comment docs.

@elacuesta elacuesta changed the title Close handler, simplify tests Closing coroutine, simplify tests Oct 10, 2021
Copy link
Contributor

@Gallaecio Gallaecio left a comment

Choose a reason for hiding this comment

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

Very nice cleanup!

elacuesta and others added 2 commits October 10, 2021 11:13
Co-authored-by: Adrián Chaves <adrian@chaves.io>
@elacuesta elacuesta closed this Oct 10, 2021
@elacuesta elacuesta reopened this Oct 10, 2021
@Gallaecio Gallaecio merged commit 190b3f3 into master Oct 10, 2021
@elacuesta elacuesta deleted the close-handler branch October 11, 2021 16:17
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.

3 participants