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

MNT Fix unit test #11438

Merged

Conversation

emteknetnz
Copy link
Member

@emteknetnz emteknetnz commented Oct 22, 2024

Issue silverstripe/.github#320

Note that while this doesn't show up in CI until CMS 6, it can be replicated on a CMS 5.3 sink install with --testsuite framework-core --filter="SecurityDefaultAdminTest|VersionedMemberAuthenticatorTest"

Fixes https://github.com/silverstripe/recipe-kitchen-sink/actions/runs/11393429390/job/31701681778

1) SilverStripe\Security\Tests\VersionedMemberAuthenticatorTest::testAuthenticateAgainstLiveStage BadMethodCallException: Object->__call(): the method 'publishSingle' does not exist on 'SilverStripe\Security\Member'

This is a hard to reproduce issue where if you run VersionedMemberAuthenticatorTest AND another test that includes $usesDatabase = true or a fixture file, and you're using kitchen-sink, you'll get the above error. However if you only run VersionedMemberAuthenticatorTest it'll be fine.

CI failure is unrelated and will be fixed in silverstripe/gha-ci#158

@GuySartorelli
Copy link
Member

For the sake of tracing this back:
This replaces #11437 which was originally part of #11432 (there's some comments there related to this broken build)

@GuySartorelli
Copy link
Member

This still feels like we're just masking a regression, so I'm going to spend 30mins or so seeing if I can find the root cause of this.

@GuySartorelli
Copy link
Member

Turns out this failure isn't new - it happens locally on 5.2 as well! Though I can't explain why it hasn't been showing in CI runs for CMS 5. I tried PHP 8.1 and 8.2 and it failed on both of those locally.

This does seem unique to some combination of unit testing and modifying the extension set at runtime - the former of which we'd just resolve somewhere in SapphireTest or one of the TestState classes and wouldn't affect production code, and the latter of which is heavily discouraged.
Because of that, I'm not spending more time on this. It still doesn't feel great, but until/unless it crops up again somewhere else I think this workaround is okay for now.

Copy link
Member

@GuySartorelli GuySartorelli left a comment

Choose a reason for hiding this comment

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

One small change

tests/php/Security/VersionedMemberAuthenticatorTest.php Outdated Show resolved Hide resolved
@emteknetnz emteknetnz force-pushed the pulls/5.3/fix-unit-test branch from a652c60 to 6df8732 Compare October 22, 2024 04:56
@GuySartorelli GuySartorelli merged commit fe39a2a into silverstripe:5.3 Oct 22, 2024
14 checks passed
@GuySartorelli GuySartorelli deleted the pulls/5.3/fix-unit-test branch October 22, 2024 05:32
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

Successfully merging this pull request may close these issues.

2 participants