configure pytest to be stricter

[graingert]

configure pytest to be stricter and fix all the problems that the increased strictness detects,

eg tests/test_datastructures::TestUploadFile PytestCollectionWarning

[JayH5]

Hey 👋 thanks for the PR, but I've gone in a slightly different direction to keep things consistent with other encode projects. I believe most of the things in this PR have now been fixed, but to summarise:

• PytestCollectionWarning on TestUploadFile: still not fixed and still worth fixing (note that an alternative to renaming the class is adding a __test__ = False attribute to it)
• "Stricter" pytest: you'd need to explain why this is desirable/what problem it solves for you.
• Trailing comma fixes for black 20 Use and pin black 20 #1042
• Regex literal in test Update CI scripts to match httpcore #1043
• Pytest config moved to file Update CI scripts to match httpcore #1043 (setup.cfg instead of pytest.ini)
• Filtering deprecation warnings in pytest: I'm not sure we actually want to do this. We probably should keep tabs on these warnings, even if they're in 3rd party libraries.

Thanks again, and I'd be open to this PR just fixing the first issue in this list, and maybe a separate PR for the second point if that's something you can give a stronger reason for.

[graingert]

"Stricter" pytest: you'd need to explain why this is desirable/what problem it solves for you.

It's very possible to accidentally skip tests by adding an __init__ method to the suite

secondly it's good to find and explicitly list the warnings that you're ignoring, rather than just having them fruitlessly print out at the end of the suite.

thirdly you had a SyntaxWarning that until run with a stricter pytest was not noticed.

[graingert]

Filtering deprecation warnings in pytest: I'm not sure we actually want to do this. We probably should keep tabs on these warnings, even if they're in 3rd party libraries.

You were previously filtering all of them, in this PR I'm fixing it so you can keep tabs on them

[JayH5]

secondly it's good to find and explicitly list the warnings that you're ignoring, rather than just having them fruitlessly print out at the end of the suite.

Fair enough, although "it's good to" is not a tangible benefit. I do see a benefit to having an explicit list as it means we know what warnings we expect and when a new one shows up we're more aware of it. On the other hand, somebody then needs to maintain the list of known warnings and it can get stale. I could go either way 🤷‍♂️

thirdly you had a SyntaxWarning that until run with a stricter pytest was not noticed.

Where was that?

---

This PR looks like a good change, thanks. IMO, parts of Starlette could do with some TLC, and using our test/lint tools to highlight those areas and keep things neat and tidy is a good idea.

Please try to keep it to a minimal set of changes, though. If the PR does exactly what it says in the title then I'm happy.

[graingert]

Where was that?

The missing r"

[graingert]

On the other hand, somebody then needs to maintain the list of known warnings and it can get stale.

Yeah it's not so bad as you get notified when you need to add new ones, I've got a ticket for xwarns decorator in pytest too

[JayH5]

If the PR does exactly what it says in the title then I'm happy.

😰 let me be clearer: that doesn't mean add more stuff to the title and then the PR is ok, it means this PR has too much stuff in it. Please try to do one thing at a time in a PR. If a PR has an "and" in the title then it's a good sign that it is doing more than one thing. pyupgrade has nothing to do with pytest and I don't see why it should be included here.

I understand that this is more work for you, but squeezing extra stuff into PRs makes more work for the reviewer. We also squash commits when we merge, so PRs that change multiple unrelated things make it harder to track down the reasons for changes in git history.

[graingert]

@JayH5 originally these are all related to the increase in strictness of pytest

[graingert]

@JayH5 I've split out the black and pyupgrade changes: #1049 #1048

[graingert]

to keep things consistent with other encode projects

I'd recommend using a custom pytest-encode plugin that configures all your settings and depends on any other pytest plugins you need across your ecosystem

[JayH5]

I'd recommend using a custom pytest-encode plugin that configures all your settings and depends on any other pytest plugins you need across your ecosystem

That might be nice to have, but unless someone has interest in building/maintaining such a thing it is unlikely to happen for now.

[JayH5]

Looks good. I've suggested a couple of comments so we can hopefully better keep track of the deprecation warnings. It looks like the context/variable ones we can maybe fix in Starlette, but for a different issue/PR.

I'm not super clear on where comments can go in setup.cfg files so relying on my syntax highlighter to say those are ok.

[JayH5]

We got there eventually 🎉 thanks for your patience