`click.get_pager_file`: add tests (#1572 followup) by AndreasBackx · Pull Request #3405 · pallets/click

AndreasBackx · 2026-05-04T19:33:28Z

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.

kdeldycke · 2026-05-05T07:35:02Z

+
+@pytest.mark.skipif(WIN, reason="Exercises the pipe pager path.")
+@pytest.mark.parametrize(
+    ("text", "color", "expected"),


I like this matrix of ANSI test cases! 👍

kdeldycke · 2026-05-05T07:39:45Z

+        yield wrapped, "utf-8", color
+
+    monkeypatch.setattr(
+        click._termui_impl, "_pager_contextmanager", pager_contextmanager


Instead of monkeypatching _pager_contextmanager everytwhere, you can just feed get_pager_file with PAGER=. That way you can check with PAGER being set to cat, PAGER left unset, PAGER set to a missing binary etc to test even more edge-cases.

Yes, I think that's the way to go because this is the top level API that is being modified by #1572. So if there are regressions, they will surface from here. Maybe you can even repack your tests to only target that higher-level API and demonstrate more use-cases, and skip testing implementation details.

And you can do the same with the other echo_via_pager function that was impacted by #1572.

I've redesigned it all a bit because I made the mistake of diving into the differences between BinaryIO, TextIO, io.StringIO, and other io shenanigans. My brain is a bit fried and I'm just going to make dinner as I'm starving.

Let me know what you think. I don't think it's perfect yet, but honestly am just gonna put this down for today and give it another look another day.

kdeldycke · 2026-05-05T07:41:56Z

+            "Popen",
+            partial(subprocess.Popen, stdout=f),
+        )
+        with force_subprocess_stdout:


Just a style nitpicking: why do you need the temporary force_subprocess_stdout variblae? Just fold everything in the with statement.

Explains a bit what it does and makes the with statement a bit less unsightly, just a preference and I don't think too much about it in test code.

kdeldycke · 2026-05-05T07:53:23Z

+    return pager_out.read_text()
+
+
+@pytest.mark.skipif(WIN, reason="Exercises the pipe pager path.")


I just learned the existence of the more utility on Windows. So you can probably test with that as a PAGER.

I've given more a try. Let's see if it passes.

…t set color unknowingly.

AndreasBackx

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

AndreasBackx · 2026-05-05T16:52:11Z

+            "Popen",
+            partial(subprocess.Popen, stdout=f),
+        )
+        with force_subprocess_stdout:


Explains a bit what it does and makes the with statement a bit less unsightly, just a preference and I don't think too much about it in test code.

AndreasBackx · 2026-05-05T18:20:19Z

+    return pager_out.read_text()
+
+
+@pytest.mark.skipif(WIN, reason="Exercises the pipe pager path.")


I've given more a try. Let's see if it passes.

AndreasBackx · 2026-05-05T18:23:05Z

+        yield wrapped, "utf-8", color
+
+    monkeypatch.setattr(
+        click._termui_impl, "_pager_contextmanager", pager_contextmanager


I've redesigned it all a bit because I made the mistake of diving into the differences between BinaryIO, TextIO, io.StringIO, and other io shenanigans. My brain is a bit fried and I'm just going to make dinner as I'm starving.

Let me know what you think. I don't think it's perfect yet, but honestly am just gonna put this down for today and give it another look another day.

Add some unit tests and integration tests for get_pager_file.

fee483d

AndreasBackx mentioned this pull request May 4, 2026

Add file-like pager: click.get_pager_file() #1572

Merged

kdeldycke changed the title ~~click.get_pager_file: add tests (#1572 followup)~~ click.get_pager_file: add tests (#1572 followup) May 5, 2026

kdeldycke added this to the 8.4.0 milestone May 5, 2026

kdeldycke added the f:prompt feature: prompt for input label May 5, 2026

kdeldycke self-requested a review May 5, 2026 07:25

kdeldycke requested changes May 5, 2026

View reviewed changes

kdeldycke reviewed May 5, 2026

View reviewed changes

kdeldycke requested a review from davidism May 5, 2026 07:53

Redesigned tests and get_pager_file branching to be more clear and no…

c995440

…t set color unknowingly.

AndreasBackx commented May 5, 2026

View reviewed changes

		return pager_out.read_text()


		@pytest.mark.skipif(WIN, reason="Exercises the pipe pager path.")

Uh oh!

Conversation

AndreasBackx commented May 4, 2026

Uh oh!

kdeldycke left a comment

Choose a reason for hiding this comment

Uh oh!

kdeldycke left a comment

Choose a reason for hiding this comment

Uh oh!

kdeldycke May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AndreasBackx left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kdeldycke May 5, 2026 •

edited

Loading