t/harness - rework App::Prove::State setup to not warn and use customizable state file #20967
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
demerphq
changed the title
khwilliamson
approved these changes
Mar 28, 2023
…izable state file
Currently we only store state if we are running parallel tests, so if
you run the tests in series we do not store data on how long they took,
and we can't use that information in a follow up parallel test run.
We also do not allow the state file to be customized to be outside of
the repo, so git clean -dfx wipes it. This means you can't keep your
test data over time, which can be a bit annoying.
We also currently construct the state object twice during setup,
resulting in two (useless) warnings when the state file is missing,
which also doubles the time to set up the tests because the yaml file
gets read twice, and not very efficiently either.
This patch changes the logic so that we initialize the state object only
once during startup, and we use the state file if we are going to run
tests, parallel or not, provided the user does not explicitly disable it
(see below). The order that tests are run is affected only when the
tests are run in parallel.
It also allows the user to specify where the state file should live,
with the $ENV{PERL_TEST_STATE_FILE} environment variable, which can be
set to 0 or the empty string to disable use of the state file if needed.
We also take care to silence the warning about an empty state file,
except in the case where the user has overriden the file name with the
$ENV{PERL_TEST_STATE_FILE}.
Lastly this patch disables loading the state data /at all/, when
the dump_tests option is invoked. There is no need nor point to
load the state data when we are simply going to dump out the list
of tests we will run.
demerphq
force-pushed
the
yves/test_harness_state_init_fixes
branch
from
24ec3b6 to
e411340
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently we only store state if we are running parallel tests, so if you run the tests in series we do not store data on how long they took, and we can't use that information in a follow up parallel test run.
We also do not allow the state file to be customized to be outside of the repo, so git clean -dfx wipes it. This means you can't keep your test data over time, which can be a bit annoying.
We also currently construct the state object twice during setup, resulting in two (useless) warnings when the state file is missing, which also doubles the time to set up the tests because the yaml file gets read twice, and not very efficiently either.
This patch changes the logic so that we initialize the state object only once during startup, and we use the state file if we are going to run tests, parallel or not, provided the user does not explicitly disable it (see below). The order that tests are run is affected only when the tests are run in parallel.
It also allows the user to specify where the state file should live, with the $ENV{PERL_TEST_STATE_FILE} environment variable, which can be set to 0 or the empty string to disable use of the state file if needed.
We also take care to silence the warning about an empty state file, except in the case where the user has overriden the file name with the $ENV{PERL_TEST_STATE_FILE}.
Lastly this patch disables loading the state data /at all/, when the dump_tests option is invoked. There is no need nor point to load the state data when we are simply going to dump out the list of tests we will run.