Skip to content

click.get_pager_file: add tests (#1572 followup)#3405

Open
AndreasBackx wants to merge 5 commits into
pallets:mainfrom
AndreasBackx:tests/get_pager_file
Open

click.get_pager_file: add tests (#1572 followup)#3405
AndreasBackx wants to merge 5 commits into
pallets:mainfrom
AndreasBackx:tests/get_pager_file

Conversation

@AndreasBackx
Copy link
Copy Markdown
Collaborator

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 kdeldycke changed the title click.get_pager_file: add tests (#1572 followup) click.get_pager_file: add tests (#1572 followup) May 5, 2026
@kdeldycke kdeldycke added this to the 8.4.0 milestone May 5, 2026
@kdeldycke kdeldycke added the f:prompt feature: prompt for input label May 5, 2026
@kdeldycke kdeldycke self-requested a review May 5, 2026 07:25
Copy link
Copy Markdown
Collaborator

@kdeldycke kdeldycke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Collaborator

@kdeldycke kdeldycke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread tests/test_termui.py Outdated
Comment thread tests/test_termui.py Outdated
Comment thread tests/test_termui.py Outdated
Comment thread tests/test_termui.py Outdated
@kdeldycke kdeldycke requested a review from davidism May 5, 2026 07:53
Copy link
Copy Markdown
Collaborator Author

@AndreasBackx AndreasBackx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread tests/test_termui.py Outdated
Comment thread tests/test_termui.py Outdated
Comment thread tests/test_termui.py Outdated
@kdeldycke
Copy link
Copy Markdown
Collaborator

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
Copy link
Copy Markdown
Collaborator

@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?

@kdeldycke kdeldycke added f:test runner feature: cli test runner windows Issues pertaining to the Windows environment labels May 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

f:prompt feature: prompt for input f:test runner feature: cli test runner windows Issues pertaining to the Windows environment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants