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

Fix TypeError when using DummyStatsCollector #4052

Merged

Conversation

elacuesta
Copy link
Member

Fixes #4007

Copy link
Member

@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.

@codecov
Copy link

codecov bot commented Oct 1, 2019

Codecov Report

Merging #4052 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #4052      +/-   ##
==========================================
+ Coverage   85.68%   85.68%   +<.01%     
==========================================
  Files         165      165              
  Lines        9732     9734       +2     
  Branches     1463     1463              
==========================================
+ Hits         8339     8341       +2     
  Misses       1136     1136              
  Partials      257      257
Impacted Files Coverage Δ
scrapy/extensions/corestats.py 100% <100%> (ø) ⬆️



if __name__ == "__main__":
unittest_main()
Copy link
Member

Choose a reason for hiding this comment

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

this looks unnecessary, tests should be selected using py.test options. I think we should be removing this code from most files.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, tox was taking too long to run a single test 😅
I'll remove it, thanks!

Copy link
Member

@Gallaecio Gallaecio Oct 2, 2019

Choose a reason for hiding this comment

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

You can use pytest parameters with tox, e.g. tox -e py36 -- tests/test_stats.py

Copy link
Member Author

Choose a reason for hiding this comment

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

tox -e py36 -- tests/test_stats.py

I use that, it takes forever anyway. I might need to read tox docs for ways to speed things up.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, right, test collection does take a bit 😓

@kmike
Copy link
Member

kmike commented Oct 1, 2019

A fix looks good to me 👍

@elacuesta elacuesta force-pushed the dummy_stats_collector_fix_elapsed_time branch from 32c96a5 to ed7af3d Compare October 1, 2019 19:11
Copy link
Member

@kmike kmike left a comment

Choose a reason for hiding this comment

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

👍 +1 to squash&merge

@elacuesta elacuesta force-pushed the dummy_stats_collector_fix_elapsed_time branch from ed7af3d to e0fabab Compare October 1, 2019 21:03
@elacuesta
Copy link
Member Author

Rebased and squashed

@kmike kmike merged commit 1650812 into scrapy:master Oct 2, 2019
@kmike
Copy link
Member

kmike commented Oct 2, 2019

Thanks @elacuesta!

@elacuesta elacuesta deleted the dummy_stats_collector_fix_elapsed_time branch October 2, 2019 14:10
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.

Exception when using DummyStatsCollector
3 participants