click.get_pager_file: add tests (#1572 followup)

[AndreasBackx]

This is a follow-up to #1572 which adds tests to validate the behaviour of click.get_pager_file. There are still some open-standing questions / todos depending on requirements:

• No Windows tests (yet) because I'm visiting family right now and only have access to a Linux machine.
  • What pager like a simple cat could we use to create simple tests? We might be able to install cat on our CI, though I haven't looked into it yet.
• What are some other edge cases we could check for? So far I've only added some simple tests:
  • Simple unit tests to test some internal implementation details. I'm not the happiest with this, but it seems like the termui testcode favours testing some internals and it was the easiest of the bunch to add.
  • Simple cat-based integration tests. We basically want to make sure that it works how we expect with a real pager, but using cat seems like the easiest way to validate this as I don't know how we'd test interactive CLIs. 🤔
  • Decide whether the current list of tests are complete enough or if we're missing anything, particularly edge cases.

[kdeldycke]

Can you add reference to that PR in the changelog along side the one from #1572?

i.e.:

-   Add ``click.get_pager_file`` for file-like access to an output
    pager. :pr:`1572` :pr:`3405`

Also update the .. versionadded:: 8.2 that was left-over from #1572.

[kdeldycke]

There are workaround and late changes in #1572 about windows so yes, it's better to try to test on that platform too. If you don't have access to a Windows machine, just remove your skip decorator and use the feedback from GitHub workflow runs. Slow, but doable.

[AndreasBackx]

Will still update the change entries, but likely not today.

[kdeldycke]

Ah. You're right. Windows runners are failing hard on more. That's probably because more is interactive and waiting user input. I guess it is safe to skip on Windows given the circumstances.

I also spotted some opportunities to extend the tests with @parametrize decorator. Let me add a commit on top of your changes to show you what I mean.

[kdeldycke]

@AndreasBackx I added a couple of tests around the PAGER env var, updated some references and re-introduced the Windows skipping behavior you originally implemented.

What do you think of these changes?

[Rowlando13]

I don't want to release 8.4.0 until these go in. @kdeldycke Feel free to keep adding small fixes and I will merge until this is ready. However, when this is ready for merge, I will accept it and just cut the release, but I expect 8.4.1 to come out shortly so I don't expect the prs to sit for long.

[kdeldycke]

I don't want to release 8.4.0 until these go in. @kdeldycke Feel free to keep adding small fixes and I will merge until this is ready. However, when this is ready for merge, I will accept it and just cut the release, but I expect 8.4.1 to come out shortly so I don't expect the prs to sit for long.

Yes, you are right to push for stale PRs and prioritize coverage now. We need a bit of effort to arrive at a final 8.4.0.

I made these changes myself on top of Andreas changes.

[kdeldycke]

I don't want to release 8.4.0 until these go in. @kdeldycke Feel free to keep adding small fixes and I will merge until this is ready. However, when this is ready for merge, I will accept it and just cut the release, but I expect 8.4.1 to come out shortly so I don't expect the prs to sit for long.

Just rebased on top of main. This is ready to be merged upstream.