Skip to content
This repository has been archived by the owner on Sep 25, 2022. It is now read-only.

Add directory or bundle loading support #15

Merged
merged 1 commit into from May 10, 2016
Merged

Add directory or bundle loading support #15

merged 1 commit into from May 10, 2016

Conversation

egeloen
Copy link
Contributor

@egeloen egeloen commented May 9, 2016

Hey!

Currently, this extension only allows to load file by file and don't provide a way to load a directory or a bundle. It would not be a problem if I don't rely on a data loader but I do and so, I can't really rely on this extension.

Then, this PR tries to add support for a directory or a bundle without breaking the BC. For bundle, I don't pass an env otherwise I would need to break the BC.

You can load a directory or a bundle with for example:

Given the following fixtures are loaded:
    | @AcmeDemoBundle    |
    | /path/to/directory |

What do you think?

$fixtureBundles[] = $this->kernel->getBundle(substr($fixturesFile, 1));
unset($fixturesFiles[$key]);
}
} else {
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not so found of the if/elseif/else, it makes it hard to read IMO:

if (0 === strpos($fixturesFile, '/') && is_dir($fixturesFile)) {
    $fixtureDirectories[] = $fixturesFile;
    unset($fixturesFiles[$key]);

    continue;
}

if (0 === strpos($fixturesFile, '@') && false === strpos($fixturesFile, '.')) {
    $fixtureBundles[] = $this->kernel->getBundle(substr($fixturesFile, 1));
    unset($fixturesFiles[$key]);

    continue;
}

$fixturesFiles[$key] = sprintf('%s/%s', $this->basePath, $fixturesFile);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can't put condition in a single if and we still need to nest the if in order to not break the logic. If a fixtures file starts with a / or an @ and is not a directory then, we must not use the base path. Anyway, I have split it in order to not rely on if/elseif/else :)

@egeloen
Copy link
Contributor Author

egeloen commented May 10, 2016

All comments have been applied!

@theofidry
Copy link
Owner

Great, thanks very much for the work @egeloen :)

@theofidry theofidry merged commit 36d94aa into theofidry:master May 10, 2016
@egeloen egeloen deleted the directory-and-bundle branch May 10, 2016 09:38
@alex62d alex62d restored the directory-and-bundle branch May 10, 2016 09:39
@egeloen egeloen deleted the directory-and-bundle branch May 10, 2016 12:19
@caciobanu caciobanu mentioned this pull request May 19, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants