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

Load all enabled apps in test bootstrap #18882

Merged
merged 1 commit into from
Sep 8, 2015
Merged

Conversation

RobinMcCorkell
Copy link
Member

A possible solution to the chaos caused by #18839. We should load all apps that have been enabled, so that autoloader valid roots are properly registered (and so that all app.php is correctly run in case there's special initialization logic). Previously, we only loaded 'minimal' apps.

Reasoning:

In autotest.sh, we enable the apps we want to test. The same happens with autotest-external.sh. The autotest executor looks at all enabled apps and runs their tests, so if an app isn't loaded it will break at this point, as the paths are not valid.

When running tests manually for individual apps (aka outside of autotest), the dev must make sure that the app is enabled in the running instance before attempting to run tests against it, OR the app test bootstrap explicitly loads the app. I don't see any situations where it is desired that an app isn't loaded, but still has tests run against it.

Please state if this is an acceptable solution @nickvergessen @oparoz @ChristophWurst

cc @MorrisJobke @LukasReschke

@RobinMcCorkell RobinMcCorkell added this to the 8.2-current milestone Sep 7, 2015
@scrutinizer-notifier
Copy link

A new inspection was created.

@RobinMcCorkell
Copy link
Member Author

TL;DR version:

For running app unit tests, either make sure you load core's bootstrap.php as per the recommended guidelines (and have the app enabled when you run your tests), or explicitly load your app in your bootstrap.php with the following line:

\OC_App::loadApp('my_awesome_app');`

@RobinMcCorkell
Copy link
Member Author

To test: try running ./autotest-external.sh mysql common-tests. Before this PR, it fails, now it succeeds

@oparoz
Copy link
Contributor

oparoz commented Sep 7, 2015

I think it makes sense for the core bootstrap since that will test the system as it's delivered, with the installed apps made available in the tests.
For apps which require a custom boostrap, adding \OC_App::loadApp is simple enough.
👍

@LukasReschke
Copy link
Member

👍

LukasReschke added a commit that referenced this pull request Sep 8, 2015
Load all enabled apps in test bootstrap
@LukasReschke LukasReschke merged commit 75f939b into master Sep 8, 2015
@LukasReschke LukasReschke deleted the load-all-apps-tests branch September 8, 2015 11:02
@lock lock bot locked as resolved and limited conversation to collaborators Aug 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants