Skip to content
This repository has been archived by the owner on Sep 30, 2024. It is now read-only.

ranking integration test #57666

Closed
Tracked by #58234
keegancsmith opened this issue Oct 17, 2023 · 6 comments · Fixed by sourcegraph/zoekt#695
Closed
Tracked by #58234

ranking integration test #57666

keegancsmith opened this issue Oct 17, 2023 · 6 comments · Fixed by sourcegraph/zoekt#695
Assignees

Comments

@keegancsmith
Copy link
Member

We need some sort of test coverage to ensure we don't regress in ranking with results from Zoekt. For example we didn't catch https://github.com/sourcegraph/sourcegraph/issues/57659 due to us not using scip-ctags in the zoekt repo. A few tests for major languages should help us a lot here.

Another idea is doing some snapshot tests on the debugscore output from zoekt. We also need to make it easy to run the same *-ctags binaries in the zoekt repo for local development.

cc @sourcegraph/search-platform

@jtibshirani
Copy link
Member

Maybe this is expanding the scope too much, but it'd be really nice to test a small set of example queries against a snapshot of the sourcegraph repo. This would not only let us detect accidental ranking changes/ regressions, but would also help test ranking tweaks in a more data-driven way. We could check that they actually help across a set of examples, instead of unexpectedly hurting some cases.

@keegancsmith
Copy link
Member Author

Maybe this is expanding the scope too much, but it'd be really nice to test a small set of example queries against a snapshot of the sourcegraph repo.

Agreed, I think this is exactly what we should do to do this task.

@keegancsmith
Copy link
Member Author

sourcegraph/zoekt#695 adds our initial integration test. Remaining work:

  • scip-ctags in the integration test.
  • update the list of queries to match what we have been testing.

@keegancsmith
Copy link
Member Author

Alright expanded the queries in sourcegraph/zoekt#697 and explored scip-ctags a little in https://github.com/sourcegraph/sourcegraph/issues/58333. I think scip-ctags work is gonna be a bit more work, but what has been done so far is very valuable and good enough to mark this issue as done.

@jtibshirani
Copy link
Member

The Zoekt tests look great! To cover the "regression testing" part of our goal, it'd also be really nice to have an end-to-end test in the sourcegraph repo that matches our production set-up as closely as possible (like using scip-ctags, using specific Zoekt options, etc.)

@keegancsmith
Copy link
Member Author

The Zoekt tests look great! To cover the "regression testing" part of our goal, it'd also be really nice to have an end-to-end test in the sourcegraph repo that matches our production set-up as closely as possible (like using scip-ctags, using specific Zoekt options, etc.)

Agreed. I already filed follow up to use scip-ctags, etc. We can adjust inside of zoekt the options used as well.

But I will add a task about a similiar test existing inside of the sourcegraph repo.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants