Skip to content

Comments

Remove python integration test in-the-past scaffolding#8086

Merged
aarongable merged 3 commits intomainfrom
rm-multistart-scaffolding
Aug 15, 2025
Merged

Remove python integration test in-the-past scaffolding#8086
aarongable merged 3 commits intomainfrom
rm-multistart-scaffolding

Conversation

@aarongable
Copy link
Contributor

@aarongable aarongable commented Mar 28, 2025

Remove the "setup_six_months_ago" and "setup_twenty_days_ago" scaffolding in our python integration test harness, which (with the removal of the last OCSP tests) no longer has any users. This simplifies that python code and reduces integration test runtime by about 30 seconds.

Part of #4934


Warning

Do not merge until #8344 has been merged

@aarongable aarongable changed the title Rm multistart scaffolding Remove python integration test in-the-past scaffolding Mar 28, 2025
@aarongable aarongable force-pushed the rm-multistart-scaffolding branch 2 times, most recently from 08ea010 to 88a6295 Compare March 31, 2025 16:15
@aarongable aarongable marked this pull request as ready for review March 31, 2025 16:55
@aarongable aarongable requested a review from a team as a code owner March 31, 2025 16:55
@aarongable aarongable requested a review from jsha March 31, 2025 16:55
@jsha
Copy link
Contributor

jsha commented Apr 2, 2025

I like the idea of saving 30 seconds on our integration tests, and I agree that this code is stable and going away soon with the removal of OCSP services. But we shouldn't land this before the actual removal of OCSP services, since we are still responsible for correct operation until then. Let's hold this PR until that happens. Other than the question of timing, looks good.

Also lol that setup_six_months_ago() uses a list comprehension and setup_twenty_days_ago() uses a for loop even though they do the exact same thing and are very close to each other. :D

@aarongable aarongable marked this pull request as draft April 17, 2025 16:09
@aarongable aarongable force-pushed the rm-multistart-scaffolding branch from 88a6295 to c1d59d0 Compare August 13, 2025 22:09
@aarongable aarongable changed the base branch from main to rm-ocsp-responder August 13, 2025 22:09
jsha
jsha previously approved these changes Aug 15, 2025
Copy link
Contributor

@jsha jsha left a comment

Choose a reason for hiding this comment

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

Looks good. It appears there is also an uncalled get_future_output that we could remove.

I assume we'll also remove the FAKECLOCK support in Boulder itself as a followup to this?

@aarongable
Copy link
Contributor Author

Good point! I'll do both of those in a followup: #8356

Base automatically changed from rm-ocsp-responder to main August 15, 2025 16:38
@aarongable aarongable dismissed jsha’s stale review August 15, 2025 16:38

The base branch was changed.

@aarongable aarongable marked this pull request as ready for review August 15, 2025 16:49
jsha
jsha approved these changes Aug 15, 2025
@beautifulentropy beautifulentropy self-requested a review August 15, 2025 19:15
@aarongable aarongable merged commit 86b9c36 into main Aug 15, 2025
12 checks passed
@aarongable aarongable deleted the rm-multistart-scaffolding branch August 15, 2025 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants