Bug 1949126 - Improve the Rust based search engine selector tests Part 2

[mandysGit]

Pull Request checklist

• Breaking changes: This PR follows our breaking change policy
  • This PR follows the breaking change policy:
    • This PR has no breaking API changes, or
    • There are corresponding PRs for our consumer applications that resolve the breaking changes and have been approved
• Quality: This PR builds and tests run cleanly
  • Note:
    • For changes that need extra cross-platform testing, consider adding [ci full] to the PR title.
    • If this pull request includes a breaking change, consider cutting a new release after merging.
• Tests: This PR includes thorough tests or an explanation of why it does not
• Changelog: This PR includes a changelog entry in CHANGELOG.md or an explanation of why it does not need one
  • Any breaking changes to Swift or Kotlin binding APIs are noted explicitly
• Dependencies: This PR follows our dependency management guidelines
  • Any new dependencies are accompanied by a summary of the due diligence applied in selecting them.

[Standard8]

Thanks for working on this. There's a few comments in-line to address.

From the patch structure, it is pretty hard to review. The problem appears to be the moves/re-ordering of the tests within the file - this makes the differences hard to read and find. It would probably have been better to do the moves of the tests in a separate commit, and then in theory the diffs would be simpler here as it would just be the actual changes.

However, as I know there's been a lot of work in this, and it is test-only, I'm not going to ask for that here, but it would be something to bear in mind in future to make it easier for reviewers.

[Standard8]

I think the changes here are fine, however I think the diffs might be a bit weird, once the other PR lands on main, please could you rebase and change this PR to be based on main, so that it is easier to check before it lands?

[mandysGit]

I think the changes here are fine, however I think the diffs might be a bit weird, once the other PR lands on main, please could you rebase and change this PR to be based on main, so that it is easier to check before it lands?

Yeah. There were merge conflicts, which I fixed here. I feel like I've either missed or added a few things.
I'll wait until the other branch is merged, then rebase this one. I'll request a re-review once this is rebased on top of main.

[Standard8]

Thank you for the updates, nice to see this getting simpler.