Skip to content

Add search tests: empty string and empty query (placeholder search) #135

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 21, 2020
Merged

Add search tests: empty string and empty query (placeholder search) #135

merged 1 commit into from
Jul 21, 2020

Conversation

eskombro
Copy link
Member

@eskombro eskombro commented Jul 21, 2020 •
edited
Loading

Closes #136

@eskombro eskombro self-assigned this Jul 21, 2020
@eskombro eskombro requested a review from curquiza July 21, 2020 15:54
curquiza
curquiza requested changes Jul 21, 2020
View reviewed changes
Copy link
Member

@curquiza curquiza left a comment

Choose a reason for hiding this comment

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

Can you add a test: a custom search with "" as q?

curquiza
curquiza requested changes Jul 21, 2020
View reviewed changes
meilisearch/tests/index/test_index_search_meilisearch.py Outdated
@@ -54,6 +66,17 @@ def test_custom_search(self):
assert '_formatted' in response['hits'][0]
assert 'dragon' in response['hits'][0]['_formatted']['title'].lower()

def test_custom_search_with_placeholder(self):
"""Tests search with no query [None] and custom parameter (attributesToHighlight)"""
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"""Tests search with no query [None] and custom parameter (attributesToHighlight)"""
"""Tests search with no query [None] and custom parameter (limit)"""

Copy link
Member Author

@eskombro eskombro Jul 21, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Hahah just saw it, thanks ;)

@eskombro eskombro force-pushed the placeholder_search branch from c213602 to a285f4d Compare July 21, 2020 16:03
@eskombro eskombro requested a review from curquiza July 21, 2020 16:04
curquiza
curquiza requested changes Jul 21, 2020
View reviewed changes
Copy link
Member

@curquiza curquiza left a comment

Choose a reason for hiding this comment

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

Sorry 😇

  • Just realized there is already a test with an empty query
  • Can you remove the {} in the second paramter: this parameter is supposed to be optional, so the user will not fill it in a basic search

@eskombro eskombro force-pushed the placeholder_search branch from a285f4d to df0d8e0 Compare July 21, 2020 16:11
@eskombro
Copy link
Member Author

eskombro commented Jul 21, 2020 •
edited
Loading

Sorry 😇

* Just realized there is already a test with an empty query

* Can you remove the `{}` in the second paramter: this parameter is supposed to be optional, so the user will not fill it in a basic search

Danke, @curquiza

  • Never too bad to have an adiditional test.
  • I removed the empty {} param from the simple request with empty query

@eskombro eskombro merged commit 981c1d6 into meilisearch-bump-v0.13.0 Jul 21, 2020
@eskombro eskombro deleted the placeholder_search branch July 21, 2020 16:14
@eskombro eskombro added the skip-changelog The PR will not appear in the release changelogs label Jul 21, 2020
@eskombro eskombro linked an issue Jul 21, 2020 that may be closed by this pull request
Placeholder search: Test #136
Closed
curquiza pushed a commit that referenced this pull request Aug 3, 2020
@eskombro @curquiza
Add search tests: empty string and empty query (placeholder search) (#…
2273d2a 
…135)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@curquiza curquiza curquiza requested changes

Assignees

@eskombro eskombro

Labels
skip-changelog The PR will not appear in the release changelogs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Placeholder search: Test
2 participants
@eskombro @curquiza