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

Add some search tests to check some fulltext searches #676

Draft
wants to merge 1 commit into
base: testing
Choose a base branch
from

Conversation

zsloan
Copy link
Contributor

@zsloan zsloan commented Mar 1, 2022

This PR adds some search tests, mainly to ensure that the fulltext searches continue to work the same way. It only checks for the correct number of results, but that should be sufficient if done across multiple possible searches.

Not sure how to actually make this test work with the CI, or whether it's even in the right place.

The tests just check that the correct number of results are returned. If
there's someething wrong with the fulltext searches, it should cause an
issue with at least one of these tests.

I'm not sure if this is the right place to put these tests, so they might need to be moved somewhere else later.
@zsloan zsloan self-assigned this Mar 1, 2022
Copy link
Collaborator

@BonfaceKilz BonfaceKilz left a comment

Choose a reason for hiding this comment

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

Also for testing, try to have these tests pass on your own local set-up. Then, add then try to capture the tests that were failing. Happy Hacking!

found = result.text.find(this_search["found_text"])
assert(found >= 0)
assert(result.status_code == 200)
print("OK")
Copy link
Collaborator

Choose a reason for hiding this comment

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

nitpick: No need for this statement.

]

def check_fulltext_searches(host):
for this_search in searches:
Copy link
Collaborator

Choose a reason for hiding this comment

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

nitpick: Try (same case for me) to be more mindful of naming. this_search can be off. How about:

Suggested change
for this_search in searches:
for search in searches:

@zsloan
Copy link
Contributor Author

zsloan commented Mar 2, 2022 via email

@BonfaceKilz
Copy link
Collaborator

BonfaceKilz commented Mar 3, 2022 via email

@arunisaac
Copy link
Contributor

arunisaac commented Mar 4, 2022 via email

@fredmanglis fredmanglis force-pushed the testing branch 2 times, most recently from 04f10df to 5474e66 Compare April 24, 2024 04:54
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.

3 participants