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

Query/Test: introduce client-side ordering to QueryTests without explicit orderby, rather than using contains in the result verification #8617

Closed
maumar opened this issue May 26, 2017 · 2 comments
Assignees
Labels
area-test closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Milestone

Comments

@maumar
Copy link
Contributor

maumar commented May 26, 2017

Currently QueryTest base (and some others) verify unordered results by using Contains. This is not perfect because it's slow and can cause false positives for queries where results have duplicates. Instead, we should apply logic similar to one in ComplexNavigationsQueryTest, where we apply client-side ordering and perform normal comparison afterwards

@maumar
Copy link
Contributor Author

maumar commented May 26, 2017

related to #8606

@maumar maumar self-assigned this May 26, 2017
@ajcvickers ajcvickers added this to the 2.0.0 milestone May 31, 2017
maumar added a commit that referenced this issue Jun 23, 2017
…ts without explicit orderby, rather than using contains in the result verification

This PR includes:
- applying client side ordering to QueryTests (rather than doing O(n^2) result comparisons for queries without order by,
- adding entry count verification (we had it in some places and not in others),
- DRYing some of the commonly used asserters

As a result, time to run those tests is cut down in half (40s to 20s). For now the helper methods are duplicated with the ones used in complex navs - will unify them in next checkin.
maumar added a commit that referenced this issue Jun 23, 2017
…ts without explicit orderby, rather than using contains in the result verification

This PR includes:
- applying client side ordering to QueryTests (rather than doing O(n^2) result comparisons for queries without order by,
- adding entry count verification (we had it in some places and not in others),
- DRYing some of the commonly used asserters

As a result, time to run those tests is cut down in half (40s to 20s). For now the helper methods are duplicated with the ones used in complex navs - will unify them in next checkin.
maumar added a commit that referenced this issue Jun 23, 2017
…ts without explicit orderby, rather than using contains in the result verification

This PR includes:
- applying client side ordering to QueryTests (rather than doing O(n^2) result comparisons for queries without order by,
- adding entry count verification (we had it in some places and not in others),
- DRYing some of the commonly used asserters

As a result, time to run those tests is cut down in half (40s to 20s). For now the helper methods are duplicated with the ones used in complex navs - will unify them in next checkin.
maumar added a commit that referenced this issue Jun 24, 2017
…ts without explicit orderby, rather than using contains in the result verification

This PR includes:
- applying client side ordering to QueryTests (rather than doing O(n^2) result comparisons for queries without order by,
- adding entry count verification (we had it in some places and not in others),
- DRYing some of the commonly used asserters

As a result, time to run those tests is cut down in half (40s to 20s). For now the helper methods are duplicated with the ones used in complex navs - will unify them in next checkin.
maumar added a commit that referenced this issue Jun 26, 2017
…ts without explicit orderby, rather than using contains in the result verification

This PR includes:
- applying client side ordering to QueryTests (rather than doing O(n^2) result comparisons for queries without order by,
- adding entry count verification (we had it in some places and not in others),
- DRYing some of the commonly used asserters

As a result, time to run those tests is cut down in half (40s to 20s). For now the helper methods are duplicated with the ones used in complex navs - will unify them in next checkin.
maumar added a commit that referenced this issue Jun 27, 2017
…ts without explicit orderby, rather than using contains in the result verification

This PR includes:
- applying client side ordering to QueryTests (rather than doing O(n^2) result comparisons for queries without order by,
- adding entry count verification (we had it in some places and not in others),
- DRYing some of the commonly used asserters

As a result, time to run those tests is cut down in half (40s to 20s). For now the helper methods are duplicated with the ones used in complex navs - will unify them in next checkin.
maumar added a commit that referenced this issue Jun 27, 2017
…ts without explicit orderby, rather than using contains in the result verification

This PR includes:
- applying client side ordering to QueryTests (rather than doing O(n^2) result comparisons for queries without order by,
- adding entry count verification (we had it in some places and not in others),
- DRYing some of the commonly used asserters

As a result, time to run those tests is cut down in half (40s to 20s). For now the helper methods are duplicated with the ones used in complex navs - will unify them in next checkin.
@maumar
Copy link
Contributor Author

maumar commented Jun 27, 2017

fixed in f45292f

@maumar maumar closed this as completed Jun 27, 2017
@maumar maumar added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Jun 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-test closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Projects
None yet
Development

No branches or pull requests

2 participants