re-enable 4 tests, accidentally dropped

[pajod]

What do these changes do?

I copied the wrong line, earlier, so those new tests did not run.
It is the (request) parser param that should have informed the xfail() decision.

 def test_request_parser_foo(parser: Any) -> None:
-    if not isinstance(response, HttpResponseParserPy):
+    if not isinstance(parser, HttpRequestParserPy):
         pytest.xfail("duh")

Are there changes in behavior for the user?

Testing only.

However, I do expect behaviour and/or documentation changes are desirable.. after I figured out a way to rewrite these tests to more clearly pinpoint the problem.

Commentary

• test_empty_header_name, which is a subset of test_invalid_header_spacing: testing against llhttp 9.2 makes those pass

• test_parse_unusual_request_line: I expected applying Fix BadStatusLine message #7651 would give me a descriptive exception text for the remaining differences, but I still get "BadStatusLine" so I stopped short of figuring out if they fail in any of the actually relevant branches, or need to be fine-tuned before even attempting at making them pass.

• test_http_request_parser_utf8_request_line: After reading up on more clever ways to abuse parser differences, maybe leaving the llhttp side as-is and making the python parser less liberal (as in rfc761#section-2.10) would be the better choice.

Related issue number

Follow-up to #8074

Checklist

• I think the code looks ugly

• Unfortunately, pytest.xfail() does not appear to share the strict_xfail behaviour of pytest.mark.xfail() - but how else could I see the XPASS when testing against llhttp main branch?

• Unit tests for the changes exist
• No API changes to be documented
• Add a new news fragment into the CHANGES/ folder
• Incomplete, but safe to merge at this stage

• While I expect no harm from merging the initial commit, going straight for a cleaner and more generic test refactoring would make more sense to me. The point of submitting is more about the verification CI run, and creating a place to leave additional comments on style and future for these tests without spamming the PR that introduced the problem

[pajod]

really should refactor how the tests with and without extensions are parametrized/generated

Originally posted by @webknjaz in #8074 (comment)

Sure glad that having PyPy in the matrix generates the the difference delta between "testing with AIOHTTP_NO_EXTENSIONS=1" and "testing aiohttp with no extensions" for free (same yarl version).

[webknjaz]

really should refactor how the tests with and without extensions are parametrized/generated

FWIW I didn't mean that you should be doing the refactoring right now. Just fixing these few places is a good starter. I wanted a more global revamp with fixtures in conftest.py, injected in all of the relevant tests. This would be a major change to the testing infra, a huge project. Before taking it on, it'd need to be talked through to synchronize the vision and stuff...

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (8b33fe6) 97.41% compared to head (bc6f24e) 97.50%.
Report is 7 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #8088      +/-   ##
==========================================
+ Coverage   97.41%   97.50%   +0.09%     
==========================================
  Files         107      107              
  Lines       32732    32745      +13     
  Branches     3823     3829       +6     
==========================================
+ Hits        31885    31928      +43     
+ Misses        640      615      -25     
+ Partials      207      202       -5     

Flag	Coverage Δ
CI-GHA	97.42% <100.00%> (+0.09%)	⬆️
OS-Linux	97.09% <100.00%> (+0.09%)	⬆️
OS-Windows	95.60% <100.00%> (+0.09%)	⬆️
OS-macOS	96.91% <100.00%> (+0.09%)	⬆️
Py-3.10.11	95.52% <100.00%> (+0.09%)	⬆️
Py-3.10.13	96.90% <100.00%> (+0.09%)	⬆️
Py-3.11.7	96.56% <69.56%> (+0.06%)	⬆️
Py-3.12.1	96.68% <69.56%> (+0.06%)	⬆️
Py-3.8.10	95.49% <100.00%> (+0.09%)	⬆️
Py-3.8.18	96.83% <100.00%> (+0.09%)	⬆️
Py-3.9.13	95.49% <100.00%> (+0.09%)	⬆️
Py-3.9.18	96.86% <100.00%> (+0.08%)	⬆️
Py-pypy7.3.15	96.40% <69.56%> (+0.06%)	⬆️
VM-macos	96.91% <100.00%> (+0.09%)	⬆️
VM-ubuntu	97.09% <100.00%> (+0.09%)	⬆️
VM-windows	95.60% <100.00%> (+0.09%)	⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Dreamsorcerer]

Oh, Pypy has an actual test error. Didn't realise at first, given that Pypy fails 50% of the time currently when installing dependencies...

[Dreamsorcerer]

@pajod Any chance you could look into the Pypy failure in order to get this PR finished?

[patchback]

Backport to 3.10: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply 0016004 on top of patchback/backports/3.10/0016004f0e5b861d35afc56a9a59040769af3122/pr-8088

Backporting merged PR #8088 into master

1. Ensure you have a local repo clone of your fork. Unless you cloned it
   from the upstream, this would be your origin remote.
2. Make sure you have an upstream repo added as a remote too. In these
   instructions you'll refer to it by the name upstream. If you don't
   have it, here's how you can add it:

   $ git remote add upstream https://github.com/aio-libs/aiohttp.git

3. Ensure you have the latest copy of upstream and prepare a branch
   that will hold the backported code:

   $ git fetch upstream
   $ git checkout -b patchback/backports/3.10/0016004f0e5b861d35afc56a9a59040769af3122/pr-8088 upstream/3.10

4. Now, cherry-pick PR re-enable 4 tests, accidentally dropped #8088 contents into that branch:

   $ git cherry-pick -x 0016004f0e5b861d35afc56a9a59040769af3122

   If it'll yell at you with something like fatal: Commit 0016004f0e5b861d35afc56a9a59040769af3122 is a merge but no -m option was given., add -m 1 as follows instead:

   $ git cherry-pick -m1 -x 0016004f0e5b861d35afc56a9a59040769af3122

5. At this point, you'll probably encounter some merge conflicts. You must
   resolve them in to preserve the patch from PR re-enable 4 tests, accidentally dropped #8088 as close to the
   original as possible.
6. Push this branch to your fork on GitHub:

   $ git push origin patchback/backports/3.10/0016004f0e5b861d35afc56a9a59040769af3122/pr-8088

7. Create a PR, ensure that the CI is green. If it's not — update it so that
   the tests and any other checks pass. This is it!
   Now relax and wait for the maintainers to process your pull request
   when they have some cycles to do reviews. Don't worry — they'll tell you if
   any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

[Dreamsorcerer]

@pajod Thanks for that. If you've got time to complete the backport as above, that'd be appreciated.

[patchback]

Backport to 3.9: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply 0016004 on top of patchback/backports/3.9/0016004f0e5b861d35afc56a9a59040769af3122/pr-8088

Backporting merged PR #8088 into master

1. Ensure you have a local repo clone of your fork. Unless you cloned it
   from the upstream, this would be your origin remote.
2. Make sure you have an upstream repo added as a remote too. In these
   instructions you'll refer to it by the name upstream. If you don't
   have it, here's how you can add it:

   $ git remote add upstream https://github.com/aio-libs/aiohttp.git

3. Ensure you have the latest copy of upstream and prepare a branch
   that will hold the backported code:

   $ git fetch upstream
   $ git checkout -b patchback/backports/3.9/0016004f0e5b861d35afc56a9a59040769af3122/pr-8088 upstream/3.9

4. Now, cherry-pick PR re-enable 4 tests, accidentally dropped #8088 contents into that branch:

   $ git cherry-pick -x 0016004f0e5b861d35afc56a9a59040769af3122

   If it'll yell at you with something like fatal: Commit 0016004f0e5b861d35afc56a9a59040769af3122 is a merge but no -m option was given., add -m 1 as follows instead:

   $ git cherry-pick -m1 -x 0016004f0e5b861d35afc56a9a59040769af3122

5. At this point, you'll probably encounter some merge conflicts. You must
   resolve them in to preserve the patch from PR re-enable 4 tests, accidentally dropped #8088 as close to the
   original as possible.
6. Push this branch to your fork on GitHub:

   $ git push origin patchback/backports/3.9/0016004f0e5b861d35afc56a9a59040769af3122/pr-8088

7. Create a PR, ensure that the CI is green. If it's not — update it so that
   the tests and any other checks pass. This is it!
   Now relax and wait for the maintainers to process your pull request
   when they have some cycles to do reviews. Don't worry — they'll tell you if
   any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

[webknjaz]

@pajod @Dreamsorcerer can this be backported to 3.9? All the test-only updates should be backported to the oldest active branch, provided they don't fail there. This is to reduce merge conflicts in future backports that might touch surrounding lines.