Skip to content

chore: Remove useless legacy autoloader for tests - #52945

Merged
skjnldsv merged 1 commit into
masterfrom
fix/cleanup-test-legacy-autoloader
May 27, 2025
Merged

chore: Remove useless legacy autoloader for tests#52945
skjnldsv merged 1 commit into
masterfrom
fix/cleanup-test-legacy-autoloader

Conversation

@come-nc

Copy link
Copy Markdown
Contributor

Summary

We do not need the legacy autoloader for the TestCase, because we have PSR-4 loading added in 54250ed

Checklist

@come-nccome-nc added this to the Nextcloud 32 milestone May 19, 2025
@come-nccome-nc self-assigned this May 19, 2025
@come-nccome-nc added the 3. to review Waiting for reviews label May 19, 2025
@come-nc
come-ncforce-pushed the fix/cleanup-test-legacy-autoloader branch 2 times, most recently from d17c564 to 9243305CompareMay 19, 2025 08:03
@come-nc
come-nc marked this pull request as ready for review May 19, 2025 08:03
@come-nc
come-nc requested a review from a team as a code ownerMay 19, 2025 08:03
@come-nc
come-nc requested review from ArtificialOwl, skjnldsv and sorbaugh and removed request for a teamMay 19, 2025 08:03
@come-nccome-nc mentioned this pull request May 19, 2025
4 tasks

@nickvergessennickvergessen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This breaks apps

@come-nc

Copy link
Copy Markdown
ContributorAuthor

This breaks apps

I expected apps were using tests/bootstrap.php, but it seems a lot have their own.

They should be updated to use:

\OC::$composerAutoloader->addPsr4('Test\\', OC::$SERVERROOT . '/tests/lib/', true);
\OC::$composerAutoloader->addPsr4('Tests\\', OC::$SERVERROOT . '/tests/', true);

As is done is server bootstrap.php.

That is still not that pretty as it depends upon \OC::$composerAutoloader which should not be public API.
Not sure what a pretty solution would be?
Maybe we add an autoload.php in tests/ that application can require?

@come-nc

come-nc commented May 19, 2025

Copy link
Copy Markdown
ContributorAuthor

@skjnldsvskjnldsv left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Once apps are handled: 👌

@nickvergessen

Copy link
Copy Markdown
Member

What about all the community apps?
https://github.com/search?q=org%3Anextcloud%20addValidRoot&type=code

@skjnldsv

Copy link
Copy Markdown
Member

What about all the community apps? github.com/search?q=org%3Anextcloud%20addValidRoot&type=code

communicate as breaking and move on ?

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
come-ncforce-pushed the fix/cleanup-test-legacy-autoloader branch from d96db5c to 9560e00CompareMay 27, 2025 14:16
@come-nc

Copy link
Copy Markdown
ContributorAuthor

@nickvergessen Can we merge now?
There is a workaround with tests/bootstrap.php and most apps migrated to it.

@skjnldsv
skjnldsv merged commit c3f16a5 into masterMay 27, 2025
@skjnldsv
skjnldsv deleted the fix/cleanup-test-legacy-autoloader branch May 27, 2025 15:57
st3iny added a commit to nextcloud/twofactor_webauthn that referenced this pull request Jun 9, 2025
Ref nextcloud/server#52945
Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
@skjnldsvskjnldsv mentioned this pull request Aug 19, 2025
@come-nccome-nc removed the pending documentation This pull request needs an associated documentation update label May 5, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to reviewWaiting for reviews

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@come-nc@nickvergessen@skjnldsv@tobiasKaminsky