Codeception / module-yii2

Codeception module for Yii2 framework
MIT License
17 stars 35 forks source link

Yii2 fixtures public API #36

Closed SamMousa closed 2 years ago

SamMousa commented 6 years ago

The Yii2 module currently has 2 ways of loading fixtures:

  1. Load them via _fixtures (or whatever is the configured name).
  2. Load them anywhere by using $I->haveFixtures().

The module has a cleanup setting that, when enabled, cleans up fixtures after a test. Loading fixtures via method 1 loads them outside a transaction. Loading fixtures via method 2 loads them inside a transaction (if transactions are enabled).

When loading fixtures they are always stored in $module->loadedFixtures, which is public (but not used anywhere outside the class).

Suppose for the rest of this issue, that neither cleanup or transactions are enabled.

  1. Can we remove $loadedFixtures from the public API? It serves no purposes and using it directly has no real uses.
  2. Instead of storing the loaded fixtures inside the module, we should just store the fact that they have been loaded. (Fixtures store reference(s) to the database component, which could either go stale or prevent connection closing).

The only use for storing loaded fixtures is doing cleanup, in which they are unset after a test. If we're not doing cleanup I think it makes sense to not store the references to the fixtures.

Resolving this issue should also make these tests pass: https://github.com/Codeception/yii2-tests/pull/1

samdark commented 6 years ago

That would be a backwards compatibility break but overall it makes sense.

Storing fixtures makes sense as well if you're using them through multiple tests in single test class.

SamMousa commented 6 years ago

Hmm, but do we unload them after all tests are done?

For me it's unclear what the "contract" is, docs suggest these fixtures are never removed.

It breaks BC because the property is public while (I think ) it never should have been.

samdark commented 6 years ago

it never should have been

It is. That's what matters.

SamMousa commented 6 years ago

I know, that's why I'm asking instead of just changing :-D anyway, it was public but without a defined behavior any change to its usage could be considered BC break.