Add FulltextContains broken tests #30907
Conversation
| 🟢 |
Run Extra TestsRun additional tests for this PR. You can customize:
|
| ⚪
🟢 |
| ⚪ ⚪ DetailsYa make output | Test bloat | Test bloat
⚪ Ya make output | Test bloat | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
There was a problem hiding this comment.
Pull request overview
This PR adds two new unit tests for the FulltextContains functionality that document currently broken behavior. According to the PR description, these tests are expected to pass but currently fail.
- Adds test for empty result scenarios when searching for non-existent text in fulltext indexes
- Adds test for queries that use fulltext index without selecting the text column in the result
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Y_UNIT_TEST(SelectWithFulltextContainsEmpty) { | ||
| auto kikimr = Kikimr(); | ||
| auto db = kikimr.GetQueryClient(); | ||
| | ||
| CreateTexts(db); | ||
| AddIndex(db); | ||
| | ||
| const auto querySettings = NYdb::NQuery::TExecuteQuerySettings().ClientTimeout(TDuration::Seconds(10)); | ||
| | ||
| { | ||
| TString query = R"sql( | ||
| SELECT `Key`, `Text` | ||
| FROM `/Root/Texts` VIEW `fulltext_idx` | ||
| WHERE FullText::FulltextContains(`Text`, "404 not found") | ||
| ORDER BY `Key`; | ||
| )sql"; | ||
| auto result = db.ExecuteQuery(query, NYdb::NQuery::TTxControl::NoTx(), querySettings).ExtractValueSync(); | ||
| UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); | ||
| CompareYson(R"([])", NYdb::FormatResultSetYson(result.GetResultSet(0))); | ||
| } | ||
| | ||
| UpsertSomeTexts(db); | ||
| | ||
| { | ||
| TString query = R"sql( | ||
| SELECT `Key`, `Text` | ||
| FROM `/Root/Texts` VIEW `fulltext_idx` | ||
| WHERE FullText::FulltextContains(`Text`, "404 not found") | ||
| ORDER BY `Key`; | ||
| )sql"; | ||
| auto result = db.ExecuteQuery(query, NYdb::NQuery::TTxControl::NoTx(), querySettings).ExtractValueSync(); | ||
| UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); | ||
| CompareYson(R"([])", NYdb::FormatResultSetYson(result.GetResultSet(0))); | ||
| } | ||
| } |
There was a problem hiding this comment.
This test is documented in the PR description as "broken" and "expected to pass but now are broken." Consider adding a comment in the test explaining this is a known failing test case and what behavior is expected. This helps future maintainers understand that the test failure is intentional until the underlying issue is fixed. For example, add a comment like: "// TODO: This test currently fails due to [issue reference]. Expected behavior is to return empty result when searching for non-existent text."
| Y_UNIT_TEST(SelectWithFulltextContainsWithoutTextField) { | ||
| auto kikimr = Kikimr(); | ||
| auto db = kikimr.GetQueryClient(); | ||
| | ||
| CreateTexts(db); | ||
| UpsertSomeTexts(db); | ||
| AddIndex(db); | ||
| | ||
| TString query = R"sql( | ||
| SELECT `Key` | ||
| FROM `/Root/Texts` VIEW `fulltext_idx` | ||
| WHERE FullText::FulltextContains(`Text`, "dog") | ||
| ORDER BY `Key`; | ||
| )sql"; | ||
| auto result = db.ExecuteQuery(query, NYdb::NQuery::TTxControl::NoTx()).ExtractValueSync(); | ||
| UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); | ||
| } |
There was a problem hiding this comment.
This test is documented in the PR description as "broken" and "expected to pass but now are broken." Consider adding a comment in the test explaining this is a known failing test case that queries without selecting the text field from the fulltext index. Additionally, the test only asserts that the query returns SUCCESS status but doesn't verify the actual result data. Consider adding an assertion to verify the expected result, such as checking that the correct Key values are returned (e.g., Key 200 which contains "dog"). For example: "// TODO: This test currently fails. Expected behavior is to successfully execute queries that only select the Key column without the Text column from fulltext index views."
| auto result = db.ExecuteQuery(query, NYdb::NQuery::TTxControl::NoTx()).ExtractValueSync(); | ||
| UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); |
There was a problem hiding this comment.
The test only verifies that the query execution status is SUCCESS but doesn't assert the actual results. Based on the test data from UpsertSomeTexts (which inserts "Dogs love foxes." with Key 200), when searching for "dog", the test should verify that Key 200 is returned. Consider adding an assertion to check the result content, similar to how other tests in the file use CompareYson to validate the result set.
stebloev force-pushed the add-fulltext-broken-tests branch from 02e5486 to 5ba5c5f Compare | ⚪
🟢 |
| ⚪ ⚪ DetailsYa make output | Test bloat | Test bloat
⚪ Ya make output | Test bloat | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
2 participants
Add FulltextContains broken tests
Changelog category
Description for reviewers
This PR adds tests that expected to pass but now are broken: empty result and query without text column.