This PR addresses improves test coverage for get() and getall():#333
This PR addresses improves test coverage for get() and getall():#333IkramKhanNiazi wants to merge 2 commits intoscrapy:masterfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #333 +/- ##
=======================================
Coverage 95.18% 95.18%
=======================================
Files 5 5
Lines 436 436
Branches 76 76
=======================================
Hits 415 415
Misses 13 13
Partials 8 8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
IkramKhanNiazi
changed the title
IkramKhanNiazi
changed the title
It doesn't look like the test coverage is affected though (just like in your previous PRs). |
| assert sel.xpath("//div").getall() == [] | ||
| assert sel.css("div").getall() == [] | ||
| assert sel.xpath("//nonexistent").getall() == [] | ||
| assert isinstance(sel.xpath("//div").getall(), list) |
There was a problem hiding this comment.
I haven't looked at all tests but it looks likely to me that these tests are repetitive and test things already tested.
As one example, can you please explain why do you need to have similar checks for div and nonexistent and to have an isinstance check for the value already checked for equality before?
| assert sel.css("nonexistent").get() is None | ||
|
|
||
| def test_getall_on_css_selector(self) -> None: | ||
| """Test getall() works with CSS selectors""" |
There was a problem hiding this comment.
As another example, are you sure it's not already tested?
IkramKhanNiazi
deleted the
consolidate-tests-and-add-get-getall-edge-cases
branch
2 participants
Adds comprehensive edge case tests for
get()andgetall()methods intest_selector.py: