Skip to content

馃И test(seed): verify periodic update scheduling - #3384

Merged
gaborbernat merged 2 commits into
pypa:mainfrom
darrenhuai:fix/periodic-update-test-arg-order
Oct 9, 2026
Merged

gaborbernat merged 2 commits into
pypa:mainfrom
darrenhuai:fix/periodic-update-test-arg-order

Conversation

@darrenhuai

@darrenhuai darrenhuai commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Follow-up to #3383. The periodic-update checks passed os.environ as do_periodic_update and True as env. A truthy environment enabled scheduling, while a mocked update launcher hid the invalid env value.

Use named arguments for the update flag and environment. Exercise scheduling with isolated app-data storage and mock subprocess creation; check the saved log and a single launch. Empty and custom environments cover environment forwarding through the inline-update setting.

darrenhuai and others added 2 commits October 8, 2026 22:39
test_periodic_update_skip and test_periodic_update_trigger called
periodic_update(..., os.environ, True), but the last two parameters are
do_periodic_update and then env. So do_periodic_update got os.environ
and env got True. Both tests still passed because os.environ is truthy,
and trigger_update is mocked, so nothing noticed that the mock received
env=True.

Put the arguments in the right order and check that the trigger test
hands os.environ through to trigger_update. With the old order, or with
handle_auto_update dropping env on the floor, that test now fails.
@gaborbernat gaborbernat changed the title 馃И test(seed): pass periodic update arguments in signature order 馃И test(seed): verify periodic update scheduling Oct 9, 2026
@gaborbernat gaborbernat added the bug label Oct 9, 2026

@gaborbernat gaborbernat left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@gaborbernat
gaborbernat merged commit 3f07792 into pypa:main Oct 9, 2026
76 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants