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

Loading from multiple roots could be unreliable #16

Open
evrcopy opened this issue Sep 22, 2020 · 2 comments
Open

Loading from multiple roots could be unreliable #16

evrcopy opened this issue Sep 22, 2020 · 2 comments

Comments

@evrcopy
Copy link

evrcopy commented Sep 22, 2020

Let's suppose i have two root folders for my tests: project/tests/unit and projects/tests/perf.

Test case:

  • I've got a test in .../unit/PRJ/src/SomeTest.php (Psr4) with fixtures .../unit/global.yml
  • I've got a test in .../perf/SomeTest.php with fixtures .../perf/global.yml
  • Both tests use @fixtures mysql write global.yml

In this case the ../unit/SomeTest.php will use fixtures from ../perf/global.yml because of implementation in src/DbFixturesTrait.php (line 288, method resolveFilePath), which is not desirable. This method favors the simpler path, not the more related one (I would expect relative paths to be checked first).

@evrcopy
Copy link
Author

evrcopy commented Sep 22, 2020

I know this could be a performance issue as well, so please elaborate.

@evrcopy
Copy link
Author

evrcopy commented Sep 22, 2020

@kambo-1st I would suggest something in the realms of:

...

    private function resolveFilePath(string $connectionName, string $filename): string {
        $this->location ??= \dirname((new \ReflectionClass($this))->getFileName()); //we would use this anyway, so better to preload here
        if ($includePaths = getenv('DB_FIXTURES_INCLUDE_PATHS_' .$connectionName)) {
            $includePaths = explode(':', $includePaths);

            foreach ($includePaths as $includePath) {
                if (
                    str_contains($this->location, $includePath) && //maybe there's some better way to check we're in the same directory root? but you get the idea
                    file_exists($includePath.$filename)
                ) {
                    return $includePath.$filename;
                }
            }
        }

        if (file_exists($filename)) {
            return $filename;
        }

        if (file_exists($filepath = $this->location . '/' . $filename)) {
            return $filepath;
        }

        throw new \InvalidArgumentException('Fixtures "' . $filename . '" not found');
    }

...

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

No branches or pull requests

1 participant