Skip to content
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

Problems with extensions FailedTestsReporter #103

Closed
rossokhinDaniil opened this issue Feb 22, 2022 · 3 comments · Fixed by #106
Closed

Problems with extensions FailedTestsReporter #103

rossokhinDaniil opened this issue Feb 22, 2022 · 3 comments · Fixed by #106

Comments

@rossokhinDaniil
Copy link

Hello! Found problems in the FailedTestsReporter extension. This extension saves the path to cases without the first character. Does this make sense or is it a bug? The problem is in this line:

return substr(str_replace($this->getRootDir(), '', $name), 1);

@jmrieger
Copy link
Contributor

@DavertMik - Would you be able to elaborate what the original intent of the substr replace was? I was trying to read the unit test, but also noted that the getTestName function was mocked in the test and isn't covered. I assume the intent is to remove the leading slash, and if that's the case, I can open a patch for it.

@reinholdfuereder
Copy link
Contributor

(Disclaimer: I have never used this code/extension; so my comment might be really useless)

https://github.com/Codeception/Codeception/blob/be148b9494dc9136bf2dea4094a78d0219bd58b9/src/Codeception/Extension.php#L145

https://github.com/Codeception/Codeception/blob/be148b9494dc9136bf2dea4094a78d0219bd58b9/src/Codeception/Configuration.php#L543

=> This really looks like there is a DIRECTORY_SEPARATOR that needs to be replaced!?

@jmrieger
Copy link
Contributor

@reinholdfuereder That's what I was seeing as well. I assume the original intent was to strip that leading DIRECTORY_SEPARATOR. I have only been working on testing this for my own use case, and I didn't see documentation of what else could be expecting that.

I suppose in either case, something like this should be a safe means to handle the separator being present (or not, as it seems to always be the case):

        return ltrim(str_replace($this->getRootDir(), '', $name), '/');

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 a pull request may close this issue.

3 participants