Revert unrolling of all()

[asottile]

Resolves #5370
Resolves #5371
Resolves #5372

[nicoddemus]

Thanks!

@Tadaboody feel free to pick this up again at a later time if you are interested in introducing this again. 👍

[asottile]

I can handle backporting of this one as well 👍 (the revert didn't apply cleanly so I imagine it'll have similar issues in 4.6-maintenance)

[codecov]

Codecov Report

Merging #5373 into master will decrease coverage by 33.39%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #5373      +/-   ##
==========================================
- Coverage   93.43%   60.04%   -33.4%     
==========================================
  Files         114       56      -58     
  Lines       25528    11072   -14456     
  Branches     2482     2048     -434     
==========================================
- Hits        23853     6648   -17205     
- Misses       1353     3775    +2422     
- Partials      322      649     +327

Impacted Files	Coverage Δ
src/_pytest/assertion/rewrite.py	81.91% <ø> (-11.07%)	⬇️
src/_pytest/junitxml.py	26.17% <0%> (-69.65%)	⬇️
src/_pytest/unittest.py	25.96% <0%> (-68.95%)	⬇️
src/_pytest/debugging.py	23.58% <0%> (-66.47%)	⬇️
src/_pytest/mark/legacy.py	32% <0%> (-65.92%)	⬇️
src/_pytest/mark/evaluate.py	32.96% <0%> (-63.7%)	⬇️
src/_pytest/pastebin.py	25.8% <0%> (-61.92%)	⬇️
src/_pytest/setuponly.py	34% <0%> (-61.75%)	⬇️
src/_pytest/stepwise.py	36.66% <0%> (-60%)	⬇️
src/_pytest/skipping.py	37% <0%> (-58.84%)	⬇️
... and 105 more

---

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 5976f36...230f736. Read the comment docs.

[blueyed]

Oh... I've thought you reverted this only for 4.6.

[RonnyPfannschmidt]

I propose never bringing this back at the ast level, there should be a difference approach

[Tadaboody]

So sorry about the fallout this created 🙏 I'll go back to the drawing board to see if this can be reintroduced in a better way. Reverting this was definitely the right decision, sorry I couldn't be more readily available to help with this

[RonnyPfannschmidt]

@Tadaboody still thanks for trying, we now know that the approach as is isn't viable

since quite a while there si the idea of assertion helper objects around, perhpas we should elaborate on it and its rammifications

[nicoddemus]

@Tadaboody not at all, as @RonnyPfannschmidt said, we appreciate you taking the time to implement this anyways! We should also have caught this earlier during review, but those things happen. 👍

[Sup3rGeo]

@nicoddemus @asottile It seems that the all unrolling was interfering with the feature #3479 I was working on, basically because this lines of code are still on the features branch:

pytest/src/_pytest/assertion/rewrite.py

Lines 920 to 921 in 4f57d40

	if self._is_any_call_with_generator_or_list_comprehension(call):
	return self._visit_all(call)

Was there any other PR merged on features with this all() unrolling or it was forgotten for removal in the features branch?

[asottile]

ah yeah it's a little confusing, for the 5.x release we're just going to release from master since there's no reason to split up the work for now -- so for now target master 👍

[nicoddemus]

Yep, sorry for the confusion!