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

Check if query is using Elasticsearch on testMultipleTests group #2400

Merged

Conversation

Rahmon
Copy link
Contributor

@Rahmon Rahmon commented Oct 12, 2021

Description of the Change

Alternate Designs

Benefits

Possible Drawbacks

Verification Process

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests passed.

Applicable Issues

#2335

Changelog Entry

@felipeelia
Copy link
Member

@Rahmon overall it looks good but it seems for some tests we are checking if the ep_wp_query_search action was fired (testWPQuerySearchContent() and testWPQuerySearchTitle()) and for the others, we are relying on $query->elasticsearch_success. Is there any difference?

@felipeelia felipeelia assigned Rahmon and unassigned felipeelia Oct 18, 2021
@Rahmon
Copy link
Contributor Author

Rahmon commented Oct 18, 2021

@Rahmon overall it looks good but it seems for some tests we are checking if the ep_wp_query_search action was fired (testWPQuerySearchContent() and testWPQuerySearchTitle()) and for the others, we are relying on $query->elasticsearch_success. Is there any difference?

You should rely only on $query->elasticsearch_success. I've updated the PR.

@Rahmon Rahmon assigned felipeelia and unassigned Rahmon Oct 18, 2021
@felipeelia felipeelia merged commit caf8841 into develop Oct 19, 2021
@felipeelia felipeelia deleted the chore/check-tests-group-testMultipleTests-issue-2335 branch October 19, 2021 16:22
@jeffpaul jeffpaul linked an issue Oct 21, 2021 that may be closed by this pull request
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.

Check if all tests are using Elasticsearch
2 participants