Skip to content

Conversation

@acoulton
Copy link
Contributor

Previously, we were running the scenario in the fixture directory itself (or explicitly copying individual files to a temporary directory). This risks Behat modifying fixture files during the scenario (potentially unexpectedly) and therefore affecting subsequent scenarios.

Instead, run every scenario in an isolated temporary working directory, and simply initialise this from the fixture at the beginning. This guarantees that every scenario runs in an isolated environment.

It also simplifies our step definitions, as we no longer need to provide for copying individual files to a temp dir or having separate steps to assert the content / presence of files in the temp dir.

Placeholders to some paths in features have changed because the paths are now relative to the %%WORKING_DIR%% placeholder (and are not nested under the fixture directory path) rather than to the project root.

This PR also adds some small refactors and enhancements to our features & FeatureContext - including fixing a bug that we were not cleaning up temporary directories created while running our features.

So that we see the behat command output if it fails, making it easier
than trying to debug why file contents do not match expectations.
These assertions were checking for deletion of the wrong file paths for
the scenario. Although the step is worded "should have been removed"
it is in practice only checking that the file does not exist - and
therefore was passing because the files had never been created.

Updated to the correct names of the files.
Previously, we were running the scenario in the fixture directory itself
(or explicitly copying individual files to a temporary directory). This
risks Behat modifying fixture files during the scenario (potentially
unexpectedly) and therefore affecting subsequent scenarios.

Instead, run every scenario in an isolated temporary working directory,
and simply initialise this from the fixture at the beginning. This
guarantees that every scenario runs in an isolated environment.

It also simplifies our step definitions, as we no longer need to provide
for copying individual files to a temp dir or having separate steps to
assert the content / presence of files in the temp dir.

Placeholders to some paths in features have changed because the paths
are now relative to the %%WORKING_DIR%% placeholder (and are not nested
under the fixture directory path) rather than to the project root.
Now that we have symfony/filesystem in dev, we can use it to simplify
creating directories and files during our own behat execution. This also
provides for more robust handling of unexpected error cases.

Using symfony/filesystem to clean up our temporary test directories also
fixes a bug that the previous ::clearDirectory method was only removing
files within the temporary directories - the directories themselves were
never removed.
Then the output should contain:
"""
Scenario: # <href=phpstorm://open?file={BASE_PATH}tests/Fixtures/EditorUrl/features/test.feature&line=3>features/test.feature:3</>
Scenario: # <href=phpstorm://open?file=%%WORKING_DIR%%features/test.feature&line=3>features/test.feature:3</>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As a separate PR, I might look at standardising / simplifying the way our FeatureContext handles the various placeholders for directories and filesystem paths - they are not quite consistent with each other, and I think some might be redundant.

But for the purpose of this PR, all paths in features will now be relative to the (previously defined) %%WORKING_DIR%% placeholder.

@acoulton
Copy link
Contributor Author

I'll take a look at the directory separator and path issues on mac/windows tomorrow.

Pending review of the best way to do this consistently / reliably across
all the features, migrate these ones to the %%DS%% placeholder for now.
The existing `"foo.php" file should contain` step passes the actual file
content through the `getExpectedOutput()` function that is also used
when asserting command output.

This function automagically converts directory separators in paths into
the DIRECTORY_SEPARATOR for the platform, as well as replacing various
other placeholders.

This fails when trying to assert the content of files generated by the
convert-config command with any `removePrefix` paths configured. The
generated files contain paths with the literal `/` character (matching
the content of the YAML) on all platforms.

Instead, assert the exact content of these files, only normalising
newlines.

Again pending review of how we consistently / reliably handle path and
directory separator issues across all our scenarios.
On MacOS and Windows runners, the path returned by sys_get_temp_dir
(and stored in the workingDir variable) is not a real path. Therefore,
it does not match when passed as an option (e.g. for --remove-prefix)
into Behat. We need to convert to a realpath so that it matches the
actual path to the files on disk.
@acoulton
Copy link
Contributor Author

@carlos-granados this should be good to go now. I will definitely take a look at how we specify / assert paths and directory separators etc in our features as a separate PR once this is merged - there's a fair bit of inconsistency that's crept in over the years and I think we could be clearer and more robust.

Since scenarios all run in a temporary working directory, and the
workingDir and tempDir are now the same, we can simplify features,
steps, and placeholders by only referring to the workingDir.
@acoulton acoulton force-pushed the maint-test-fixture-dirs branch from e769740 to ca9d65d Compare August 27, 2025 10:34
Copy link
Contributor

@carlos-granados carlos-granados left a comment

Choose a reason for hiding this comment

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

Thanks @acoulton, great job, just a tiny comment, feel free to ignore it

@acoulton acoulton merged commit 036e638 into Behat:master Aug 29, 2025
19 checks passed
@acoulton acoulton deleted the maint-test-fixture-dirs branch August 29, 2025 07:35
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