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

[stable25] optimize calendar search query #39787

Merged
merged 2 commits into from
Aug 11, 2023

Conversation

backportbot-nextcloud[bot]
Copy link

Backport of #39741

@ChristophWurst ChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Aug 10, 2023
@AndyScherzinger
Copy link
Member

@ChristophWurst are the CI failures relevant?

@ChristophWurst
Copy link
Member

@ChristophWurst
Copy link
Member

1) OCA\DAV\Tests\unit\CalDAV\CalDavBackendTest::testSearchPrincipal
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
 END:VTIMEZONE\n
 BEGIN:VEVENT\n
 DTSTART;TZID=Europe/Berlin:20160419T130000\n
-SUMMARY:My Test (public)\n
-CLASS:PUBLIC\n
+SUMMARY:My Test (confidential)\n
+CLASS:CONFIDENTIAL\n
 TRANSP:OPAQUE\n
 STATUS:CONFIRMED\n
 DTEND;TZID=Europe/Berlin:20160419T140000\n
@@ @@
 LAST-MODIFIED:20160419T074202Z\n
 DTSTAMP:20160419T074202Z\n
 CREATED:20160419T074202Z\n
-UID:2e468c48-7860-492e-bc52-92fa0daeeccf.1461051722310-1\n
+UID:2e468c48-7860-492e-bc52-92fa0daeeccf.1461051722310-3\n
 END:VEVENT\n
 END:VCALENDAR'

/drone/src/apps/dav/tests/unit/CalDAV/CalDavBackendTest.php:1277

@ChristophWurst
Copy link
Member

Interesting enough the test succeded on master and stable26

@AndyScherzinger AndyScherzinger added 3. to review Waiting for reviews and removed 4. to release Ready to be released and/or waiting for tests to finish labels Aug 10, 2023
@tcitworld
Copy link
Member

Looks like the returned order changed, however since there's no orderBy happening in these requests it could just be a race condition having made the insertion in the wrong order, which is likely since the test only failed on pg10php7.4.

Do you mind retrying it to check if it's indeed flaky?

@ChristophWurst
Copy link
Member

@AndyScherzinger could you please restart the drone run https://drone.nextcloud.com/nextcloud/server/38388? I lack permissions

@AndyScherzinger
Copy link
Member

@AndyScherzinger could you please restart the drone run https://drone.nextcloud.com/nextcloud/server/38388? I lack permissions

DONE @ChristophWurst

@tcitworld
Copy link
Member

Job passed this time 🤷

@ChristophWurst
Copy link
Member

So nothing is broken, yay. But it means that the test might be a bit flaky and fail occasionally in the future.

@AndyScherzinger AndyScherzinger merged commit e01c37d into stable25 Aug 11, 2023
29 checks passed
@AndyScherzinger AndyScherzinger deleted the backport/39741/stable25 branch August 11, 2023 09:24
@AndyScherzinger
Copy link
Member

Merged in alignment with Christoph, there might be a follow-up PR for stabilizing the flaky test.

@blizzz blizzz mentioned this pull request Sep 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants