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

Tests for email collaborators pagination #8710

Merged
merged 2 commits into from
Mar 9, 2018

Conversation

nickvergessen
Copy link
Member

Followup to #8596

@@ -196,7 +200,7 @@ public function search($search, $limit, $offset, ISearchResult $searchResult) {
}
$searchResult->addResultSet($emailType, $result['wide'], $result['exact']);

return true;
Copy link
Member Author

Choose a reason for hiding this comment

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

this was a pretty bad assumption, so I had to fix all tests and wrote some where it is actually supposed to be true

Copy link
Member

Choose a reason for hiding this comment

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

Its the default for anything that cannot (or could not) paginate. I took it over as it was when i refactored it.

@blizzz
Copy link
Member

blizzz commented Mar 7, 2018

no test case where you look for $reachedEnd = true?

if (!$this->shareeEnumeration) {
$result['wide'] = [];
$userResults['wide'] = [];
} else {
$reachedEnd = (count($result['wide']) < $offset + $limit) ||
Copy link
Member

Choose a reason for hiding this comment

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

Isn't the end reached when both sets don't have further elements?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah should have been &&

also the answer for @blizzz comment. I messed up when inverting the logic from "more results" to "end reached"

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@nickvergessen nickvergessen force-pushed the followup/8596/tests-for-user-pagination branch from db8d664 to d03663c Compare March 7, 2018 11:12
@blizzz
Copy link
Member

blizzz commented Mar 8, 2018

failing tests

Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen nickvergessen force-pushed the followup/8596/tests-for-user-pagination branch from d03663c to fe30d63 Compare March 8, 2018 12:26
@codecov
Copy link

codecov bot commented Mar 8, 2018

Codecov Report

Merging #8710 into master will decrease coverage by 16.88%.
The diff coverage is 0%.

@@              Coverage Diff              @@
##             master    #8710       +/-   ##
=============================================
- Coverage     51.86%   34.98%   -16.89%     
- Complexity    25373    25374        +1     
=============================================
  Files          1607     1607               
  Lines         95170    95173        +3     
  Branches       1379     1379               
=============================================
- Hits          49362    33297    -16065     
- Misses        45808    61876    +16068
Impacted Files Coverage Δ Complexity Δ
...private/Collaboration/Collaborators/MailPlugin.php 0% <0%> (-75%) 29 <0> (+1)
...ty/Exceptions/CrossSiteRequestForgeryException.php 0% <0%> (-100%) 1% <0%> (ø)
lib/private/Hooks/ForwardingEmitter.php 0% <0%> (-100%) 5% <0%> (ø)
...mments/tests/Unit/Controller/NotificationsTest.php 0% <0%> (-100%) 4% <0%> (ø)
...ddleware/Security/Exceptions/NotAdminException.php 0% <0%> (-100%) 1% <0%> (ø)
...pFramework/Db/MultipleObjectsReturnedException.php 0% <0%> (-100%) 1% <0%> (ø)
apps/dav/lib/CalDAV/PublicCalendarObject.php 0% <0%> (-100%) 1% <0%> (ø)
apps/user_ldap/lib/Settings/Section.php 0% <0%> (-100%) 5% <0%> (ø)
apps/files/appinfo/app.php 0% <0%> (-100%) 0% <0%> (ø)
lib/private/App/CodeChecker/EmptyCheck.php 0% <0%> (-100%) 6% <0%> (ø)
... and 523 more

@nickvergessen
Copy link
Member Author

I messed up conflict resolution on rebase.
Fixed now

Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen nickvergessen force-pushed the followup/8596/tests-for-user-pagination branch from fe30d63 to 8820966 Compare March 8, 2018 15:22
@nickvergessen
Copy link
Member Author

Backport in #8749

@blizzz blizzz merged commit 054a45c into master Mar 9, 2018
@blizzz blizzz deleted the followup/8596/tests-for-user-pagination branch March 9, 2018 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants