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.

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

Move continuation token vespa search tests to separate file #336

Merged
merged 9 commits into from
Sep 17, 2024
Merged

Move continuation token vespa search tests to separate file #336

merged 9 commits into from
Sep 17, 2024

Conversation

katybaulch
Copy link
Contributor

Description

When working on PDCT-1274, I found that the search test files are huge and kinda grim to maintain. This PR splits the vespa search continuation token tests into a separate file as part five of a refactor to make the search tests more maintainable.

Proposed version

Please select the option below that is most relevant from the list below. This
will be used to generate the next tag version name during auto-tagging.

  • Skip auto-tagging
  • Patch
  • Minor version
  • Major version

Visit the Semver website to understand the
difference between MAJOR, MINOR, and PATCH versions.

Notes:

  • If none of these options are selected, auto-tagging will fail
  • Where multiple options are selected, the most senior option ticked will be
    used -- e.g. Major > Minor > Patch
  • If you are selecting the version in the list above using the textbox, make
    sure your selected option is marked [x] with no spaces in between the
    brackets and the x

Type of change

Please select the option(s) below that are most relevant:

  • Bug fix
  • New feature
  • Breaking change
  • GitHub workflow update
  • Documentation update
  • Refactor legacy code
  • Dependency update

How Has This Been Tested?

Please describe the tests that you added to verify your changes.

Reviewer Checklist

  • DB_CLIENT DEPENDENCY IS ON THE LATEST VERSION
  • The PR represents a single feature (small driveby fixes are also ok)
  • The PR includes tests that are sufficient for the level of risk
  • The code is sufficiently commented, particularly in hard-to-understand areas
  • Any required documentation updates have been made
  • Any TODOs added are captured in future tickets
  • No FIXMEs remain

@katybaulch katybaulch requested a review from a team as a code owner September 17, 2024 12:25
odrakes-cpr
odrakes-cpr approved these changes Sep 17, 2024
View reviewed changes
Copy link
Contributor

@odrakes-cpr odrakes-cpr left a comment

Choose a reason for hiding this comment

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

lgtm

olaughter
olaughter approved these changes Sep 17, 2024
View reviewed changes
Copy link
Contributor

@olaughter olaughter left a comment

Choose a reason for hiding this comment

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

Love to see it! One small suggestion below

tests/search/vespa/test_vespa_search_cont_tokens.py Outdated Show resolved Hide resolved
@katybaulch katybaulch merged commit 6dc4c7f into main Sep 17, 2024
8 checks passed
@katybaulch katybaulch deleted the move-cont-token-tests-to-separate-file branch September 17, 2024 12:48
THOR300 added a commit that referenced this pull request Sep 17, 2024
@THOR300 @katybaulch
Feature/resolve merge conflict v2 (#340)
75d0410 
* Move vespa search tests & the search_fixtures they use under dedicated sub-folder  (#334)

* Move vespa search tests under dedicated vespa folder

* Move /search_fixtures under vespa search folder & rename to fixtures

* Bump to 1.14.20

* Move vespa search result order tests into a separate file (#335)

* Move vespa search tests under dedicated vespa folder

* Move /search_fixtures under vespa search folder & rename to fixtures

* Bump to 1.14.20

* Move vespa search result order tests to separate file

* Bump to 1.14.19

* Move continuation token vespa search tests to separate file (#336)

* Move vespa search tests under dedicated vespa folder

* Move /search_fixtures under vespa search folder & rename to fixtures

* Bump to 1.14.20

* Move vespa search result order tests to separate file

* Bump to 1.14.19

* Move vespa search continuation token tests to separate file

* Group pagination and continuation token tests

* Move keyword and range vespa search tests into separate file (#337)

* Move vespa search tests under dedicated vespa folder

* Move /search_fixtures under vespa search folder & rename to fixtures

* Bump to 1.14.20

* Move vespa search result order tests to separate file

* Bump to 1.14.19

* Move vespa search continuation token tests to separate file

* Move keyword and range vespa search tests into separate file

* Delete test_vespa_search_cont_tokens.py

* Move _make_search_request into vespa search setup

* Move vespa search tests for ignoring special chars & case to separate file (#338)

* Move data download tests into parent folder

* Move query insensitivity & special chars ignoring tests out

* Rename from test_vespasearch

* Bump to 1.14.20

* Removing refactored file.

* Adding back in the changes from the test_vespasearch.

---------

Co-authored-by: Katy Baulch <[email protected]>
Co-authored-by: Mark <[email protected]>
THOR300 added a commit that referenced this pull request Sep 17, 2024
@THOR300 @katybaulch
Surfacing mcf required filters in the backend (#327)
a577fa9 
* Updating the cpr_sdk version.

* Updating the version in the pyproject.toml.

* Adding todo comments.

* Pushing working changes.

* Pushing working changes.

* Updating vespa search params test.

* Correcting the browse test fixtures.

* Updating convert filters test.

* Removing todo.

* Updating browse functionality to return geographies.

* updating the search tests.

* Adding a WIP commit.

* Attempting trunk fix.

* Down to eight failures with this reversion.

* Bugfix for the test_no_doc_if_in_postgres_but_not_vespa test.

* Bumping the cpr_sdk version.

* Cleaning up.

* Removing from test.

* Adding metadata and corpus checks to the tests.

* Feature/add a test for metadata (#331)

* Working commit.

* Clean up.

* Making populate db families deterministic.

* Adding distinct deterministic and random fixtures for family metadata.

---------

Co-authored-by: Mark <[email protected]>

* Trunk fix.

* Feature/resolve merge conflict v2 (#340)

* Move vespa search tests & the search_fixtures they use under dedicated sub-folder  (#334)

* Move vespa search tests under dedicated vespa folder

* Move /search_fixtures under vespa search folder & rename to fixtures

* Bump to 1.14.20

* Move vespa search result order tests into a separate file (#335)

* Move vespa search tests under dedicated vespa folder

* Move /search_fixtures under vespa search folder & rename to fixtures

* Bump to 1.14.20

* Move vespa search result order tests to separate file

* Bump to 1.14.19

* Move continuation token vespa search tests to separate file (#336)

* Move vespa search tests under dedicated vespa folder

* Move /search_fixtures under vespa search folder & rename to fixtures

* Bump to 1.14.20

* Move vespa search result order tests to separate file

* Bump to 1.14.19

* Move vespa search continuation token tests to separate file

* Group pagination and continuation token tests

* Move keyword and range vespa search tests into separate file (#337)

* Move vespa search tests under dedicated vespa folder

* Move /search_fixtures under vespa search folder & rename to fixtures

* Bump to 1.14.20

* Move vespa search result order tests to separate file

* Bump to 1.14.19

* Move vespa search continuation token tests to separate file

* Move keyword and range vespa search tests into separate file

* Delete test_vespa_search_cont_tokens.py

* Move _make_search_request into vespa search setup

* Move vespa search tests for ignoring special chars & case to separate file (#338)

* Move data download tests into parent folder

* Move query insensitivity & special chars ignoring tests out

* Rename from test_vespasearch

* Bump to 1.14.20

* Removing refactored file.

* Adding back in the changes from the test_vespasearch.

---------

Co-authored-by: Katy Baulch <[email protected]>
Co-authored-by: Mark <[email protected]>

---------

Co-authored-by: Mark <[email protected]>
Co-authored-by: Katy Baulch <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@olaughter olaughter olaughter approved these changes

@odrakes-cpr odrakes-cpr odrakes-cpr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@katybaulch @olaughter @odrakes-cpr