Skip to content

Ruleset::processRuleset(): add tests for handling <autoload> directives #715

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 2, 2024

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Nov 22, 2024

Description

Add tests specifically aimed at verifying <autoload> elements are read and handled correctly.

Note: as autoload directives are not stored in the ruleset, but directly lead to a file include using include_once, these tests need to run in a separate process to circumvent the possibility of test cross-contamination.

Suggested changelog entry

N/A

Related issues/external references

This PR is part of a series of PRs expanding the tests for the Ruleset class.

…ives

Add tests specifically aimed at verifying `<autoload>` elements are read and handled correctly.

Note: as `autoload` directive are not stored in the ruleset, but directly lead to a file include using `include_once`, these tests need to run in a separate process to circumvent the possibility of test cross-contamination.
Copy link
Contributor

@rodrigoprimo rodrigoprimo left a comment

Choose a reason for hiding this comment

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

LGTM!

I just wonder if you considered adding a path to the XML file that triggers the code on the line below:

$autoloadPath = $relativePath;

It seems to me that all the provided paths are making Common:realpath() returns false, and so the line that I mentioned is never executed.

@jrfnl
Copy link
Member Author

jrfnl commented Nov 29, 2024

I just wonder if you considered adding a path to the XML file that triggers the code on the line below:

$autoloadPath = $relativePath;

It seems to me that all the provided paths are making Common:realpath() returns false, and so the line that I mentioned is never executed.

Probably won't surprise you that I did consider it, I just needed some time to investigate what was going on a little more. Done that now, see the new commit.

@rodrigoprimo rodrigoprimo self-requested a review December 2, 2024 14:32
@jrfnl
Copy link
Member Author

jrfnl commented Dec 2, 2024

Thanks for reviewing this @rodrigoprimo !

@jrfnl jrfnl merged commit c942e6a into master Dec 2, 2024
57 checks passed
@jrfnl jrfnl deleted the feature/ruleset-add-tests-process-ruleset-autoload branch December 2, 2024 21:55
jrfnl added a commit that referenced this pull request Dec 2, 2024
…ives (#715)

Add tests specifically aimed at verifying `<autoload>` elements are read and handled correctly.

Note: as `autoload` directive are not stored in the ruleset, but directly lead to a file include using `include_once`, these tests need to run in a separate process to circumvent the possibility of test cross-contamination.
jrfnl added a commit that referenced this pull request Dec 2, 2024
…ives (#715)

Add tests specifically aimed at verifying `<autoload>` elements are read and handled correctly.

Note: as `autoload` directive are not stored in the ruleset, but directly lead to a file include using `include_once`, these tests need to run in a separate process to circumvent the possibility of test cross-contamination.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants