feat(clp-s): Add end to end search tests for clp-s. #668
Conversation
WalkthroughThis pull request introduces substantial enhancements to the CLP (Compressed Log Processing) project, focusing on expanding search functionality, testing infrastructure, and build configuration. The changes include adding numerous source files for search operations, timestamp handling, and reducer components, updating the CMakeLists.txt build configuration, and introducing new test utilities. The modifications extend the project's capabilities in log processing, search transformations, and testing methodologies without disrupting existing functionalities. Changes
Sequence DiagramsequenceDiagram
participant SR as Search Request
participant TF as Transformation Passes
participant AR as ArchiveReader
participant SI as Search Index
participant OH as OutputHandler
SR->>TF: Apply standardization
TF->>TF: Narrow types
TF->>TF: Convert to existence
TF->>AR: Process archives
AR->>SI: Evaluate timestamp index
SI->>AR: Filter resu
10000
lts
AR->>OH: Write matching results
Possibly Related PRs
Suggested Reviewers
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
components/core/tests/test_log_files/test_search.jsonl (1)
1-9: Consider adding more comprehensive test cases.The test data covers basic scenarios but could benefit from additional test cases to ensure robust search functionality:
- Empty strings and null values
- Unicode characters and emojis
- Deeply nested objects and arrays
- Very large strings to test size limits
- Special JSON characters like
\u0000components/core/tests/test-clp_s-end_to_end.cpp (1)
Line range hint
39-61: Extract magic numbers into named constants.The size constants should be extracted into named constants at class/namespace scope for better maintainability.
+namespace { +constexpr auto cGiB = 1024ULL * 1024 * 1024; +constexpr auto cMiB = 1024ULL * 1024; +constexpr auto cDefaultTargetEncodedSize = 8ULL * cGiB; +constexpr auto cDefaultMaxDocumentSize = 512ULL * cMiB; +constexpr auto cDefaultMinTableSize = 1ULL * cMiB; +} // namespace void compress(bool structurize_arrays, bool single_file_archive) { - constexpr auto cDefaultTargetEncodedSize = 8ULL * 1024 * 1024 * 1024; // 8 GiB - constexpr auto cDefaultMaxDocumentSize = 512ULL * 1024 * 1024; // 512 MiB - constexpr auto cDefaultMinTableSize = 1ULL * 1024 * 1024; // 1 MiBcomponents/core/tests/test-clp_s-search.cpp (4)
49-58: Add error handling for path operations.While the path utility functions are well-implemented, they should handle cases where the test input file doesn't exist.
auto get_test_input_local_path() -> std::string { std::filesystem::path const current_file_path{__FILE__}; auto const tests_dir{current_file_path.parent_path()}; - return (tests_dir / get_test_input_path_relative_to_tests_dir()).string(); + auto const input_path = tests_dir / get_test_input_path_relative_to_tests_dir(); + if (!std::filesystem::exists(input_path)) { + throw std::runtime_error(fmt::format("Test input file not found: {}", input_path.string())); + } + return input_path.string(); }
60-63: Extract magic numbers as named constants.The configuration values should be defined as named constants at the top of the file for better maintainability.
+constexpr auto cDefaultTargetEncodedSizeGiB = 8ULL; +constexpr auto cDefaultMaxDocumentSizeMiB = 512ULL; +constexpr auto cDefaultMinTableSizeMiB = 1ULL; +constexpr auto cDefaultCompressionLevel = 3; + - constexpr auto cDefaultTargetEncodedSize = 8ULL * 1024 * 1024 * 1024; // 8 GiB - constexpr auto cDefaultMaxDocumentSize = 512ULL * 1024 * 1024; // 512 MiB - constexpr auto cDefaultMinTableSize = 1ULL * 1024 * 1024; // 1 MiB - constexpr auto cDefaultCompressionLevel = 3; + constexpr auto cDefaultTargetEncodedSize = cDefaultTargetEncodedSizeGiB * 1024 * 1024 * 1024; + constexpr auto cDefaultMaxDocumentSize = cDefaultMaxDocumentSizeMiB * 1024 * 1024; + constexpr auto cDefaultMinTableSize = cDefaultMinTableSizeMiB * 1024 * 1024;
110-166: Enhance error messages and add documentation.The search function would benefit from more descriptive error messages and documentation of the transformation workflow.
+/** + * Executes a search query and validates the results. + * + * Workflow: + * 1. Parse KQL expression + * 2. Apply transformation passes: + * - Standardize to OR-of-AND form + * - Narrow types + * - Convert to exists + * 3. Process archives and collect results + * 4. Validate results against expected outcomes + */ void search(std::string const& query, bool ignore_case, std::vector<int64_t> const& expected_results) { - REQUIRE(expected_results.size() > 0); + REQUIRE_MESSAGE(expected_results.size() > 0, "Expected results vector cannot be empty"); auto query_stream = std::istringstream{query}; auto expr = clp_s::search::kql::parse_kql_expression(query_stream); - REQUIRE(nullptr != expr); - REQUIRE(nullptr == std::dynamic_pointer_cast<clp_s::search::EmptyExpr>(expr)); + REQUIRE_MESSAGE(nullptr != expr, fmt::format("Failed to parse query: {}", query)); + REQUIRE_MESSAGE(nullptr == std::dynamic_pointer_cast<clp_s::search::EmptyExpr>(expr), + fmt::format("Query resulted in empty expression: {}", query));
175-177: Track the skipped test case.The commented-out test case should be tracked as a GitHub issue for follow-up.
Would you like me to create a GitHub issue to track the bug in
Grep::process_raw_queryand this skipped test case?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
components/core/CMakeLists.txt(6 hunks)components/core/src/clp_s/search/OutputHandler.hpp(2 hunks)components/core/tests/LogSuppressor.hpp(2 hunks)components/core/tests/TestOutputCleaner.hpp(1 hunks)components/core/tests/test-clp_s-end_to_end.cpp(5 hunks)components/core/tests/test-clp_s-search.cpp(1 hunks)components/core/tests/test_log_files/test_search.jsonl(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
components/core/tests/TestOutputCleaner.hpp (1)
Pattern **/*.{cpp,hpp,java,js,jsx,ts,tsx}: - Prefer false == <expression> rather than !<expression>.
components/core/tests/LogSuppressor.hpp (1)
Pattern **/*.{cpp,hpp,java,js,jsx,ts,tsx}: - Prefer false == <expression> rather than !<expression>.
components/core/tests/test-clp_s-search.cpp (1)
Pattern **/*.{cpp,hpp,java,js,jsx,ts,tsx}: - Prefer false == <expression> rather than !<expression>.
components/core/tests/test-clp_s-end_to_end.cpp (1)
Pattern **/*.{cpp,hpp,java,js,jsx,ts,tsx}: - Prefer false == <expression> rather than !<expression>.
components/core/src/clp_s/search/OutputHandler.hpp (1)
Pattern **/*.{cpp,hpp,java,js,jsx,ts,tsx}: - Prefer false == <expression> rather than !<expression>.
📓 Learnings (1)
components/core/tests/test-clp_s-end_to_end.cpp (1)
Learnt from: AVMatthews
PR: y-scope/clp#595
File: components/core/tests/test-end_to_end.cpp:59-65
Timestamp: 2024-11-19T17:30:04.970Z
Learning: In 'components/core/tests/test-end_to_end.cpp', during the 'clp-s_compression_and_extraction_no_floats' test, files and directories are intentionally removed at the beginning of the test to ensure that any existing content doesn't influence the test results.
🪛 cppcheck (2.10-2)
components/core/tests/test-clp_s-search.cpp
[error] 191-191: Exception thrown in function declared not to throw exceptions.
(throwInNoexceptFunction)
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: ubuntu-focal-static-linked-bins
- GitHub Check: ubuntu-jammy-static-linked-bins
- GitHub Check: centos-stream-9-static-linked-bins
- GitHub Check: ubuntu-focal-dynamic-linked-bins
- GitHub Check: ubuntu-jammy-dynamic-linked-bins
- GitHub Check: centos-stream-9-dynamic-linked-bins
- GitHub Check: build-macos (macos-14, false)
- GitHub Check: build-macos (macos-14, true)
- GitHub Check: lint-check (ubuntu-latest)
- GitHub Check: build-macos (macos-13, false)
- GitHub Check: build-macos (macos-13, true)
- GitHub Check: lint-check (macos-latest)
🔇 Additional comments (7)
components/core/tests/LogSuppressor.hpp (1)
18-22: LGTM! Well-structured RAII implementation.The deletion of copy/move operations prevents potential issues with logging level management. The trailing return type syntax is used consistently.
components/core/tests/test-clp_s-end_to_end.cpp (2)
130-134: LGTM! Proper cleanup initialization.The TestOutputCleaner is correctly initialized with all necessary paths, ensuring proper cleanup before and after tests.
Line range hint
95-121: Enhance shell command security and cross-platform compatibility.The shell commands should properly escape paths and handle spaces. Consider using a cross-platform solution.
- auto command = fmt::format( - "jq --sort-keys --compact-output '.' {} | sort > {}", - extracted_json_path.string(), - cTestEndToEndOutputSortedJson - ); + auto command = fmt::format( + "jq --sort-keys --compact-output '.' {} | sort > {}", + std::filesystem::path(extracted_json_path).make_preferred().string(), + std::filesystem::path(cTestEndToEndOutputSortedJson).make_preferred().string() + );Also, verify the commands exist before using them:
components/core/tests/test-clp_s-search.cpp (2)
1-37: LGTM! Well-organized includes and constants.The includes are logically grouped, and constants follow a consistent naming convention.
38-48: LGTM! Well-designed function declarations.Functions are appropriately scoped in an anonymous namespace and have clear signatures using const references.
components/core/src/clp_s/search/OutputHandler.hpp (1)
298-345: LGTM! Well-designed output handler implementation.The VectorOutputHandler class is well-implemented with:
- Appropriate use of const references
- Clear member variable ownership
- Consistent interface implementation
components/core/CMakeLists.txt (1)
Line range hint
302-664: LGTM! Well-organized build configuration updates.The changes maintain a consistent style and clear organization of source files.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
components/core/tests/test-clp_s-search.cpp (3)
100-103: Enhance error message with exception details.Include the exception message in the failure output for better debugging.
- FAIL(fmt::format("Invalid JSON in result: {}", result.message)); + FAIL(fmt::format("Invalid JSON in result: {}. Error: {}", result.message, e.what()));
117-127: Document transformation passes.Add comments explaining the purpose of each transformation pass for better maintainability.
+ // Convert the expression to a standardized form (OR of ANDs) clp_s::search::OrOfAndForm standardize_pass; expr = standardize_pass.run(expr); REQUIRE(nullptr != expr); + // Narrow down the types in the expression for more efficient processing clp_s::search::NarrowTypes narrow_pass; expr = narrow_pass.run(expr); REQUIRE(nullptr != expr); + // Convert applicable expressions to existence checks clp_s::search::ConvertToExists convert_pass; expr = convert_pass.run(expr); REQUIRE(nullptr != expr);
130-162: Use RAII for archive reader resources.Consider using a unique_ptr or scope guard to ensure archive reader is closed even if an exception occurs.
+ class ArchiveReaderGuard { + public: + explicit ArchiveReaderGuard(std::shared_ptr<clp_s::ArchiveReader> reader) + : reader_(std::move(reader)) {} + ~ArchiveReaderGuard() { + if (reader_) { + reader_->close(); + } + } + auto get() const -> clp_s::ArchiveReader* { return reader_.get(); } + private: + std::shared_ptr<clp_s::ArchiveReader> reader_; + }; for (auto const& entry : std::filesystem::directory_iterator(cTestSearchArchiveDirectory)) { auto archive_reader = std::make_shared<clp_s::ArchiveReader>(); + ArchiveReaderGuard guard(archive_reader); // ... rest of the code ... - archive_reader->close(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/core/tests/test-clp_s-search.cpp(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
components/core/tests/test-clp_s-search.cpp (1)
Pattern **/*.{cpp,hpp,java,js,jsx,ts,tsx}: - Prefer false == <expression> rather than !<expression>.
🪛 cppcheck (2.10-2)
components/core/tests/test-clp_s-search.cpp
[error] 191-191: Exception thrown in function declared not to throw exceptions.
(throwInNoexceptFunction)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: build-macos (macos-14, false)
- GitHub Check: build-macos (macos-13, false)
- GitHub Check: build-macos (macos-13, true)
- GitHub Check: build (macos-latest)
- GitHub Check: lint-check (ubuntu-latest)
- GitHub Check: lint-check (macos-latest)
🔇 Additional comments (4)
components/core/tests/test-clp_s-search.cpp (4)
1-37: LGTM! Well-organized headers and constants.The headers are logically organized, and constants follow a consistent naming convention with clear prefixes.
38-48: LGTM! Well-structured function declarations.Good use of anonymous namespace for test-only functions and modern C++ conventions with trailing return types.
66-67: Improve directory creation error handling.The directory creation should be wrapped in a try-catch block to ensure proper cleanup on failure.
189-191: Handle potential exceptions in the test loop.The static analysis tool indicates that exceptions might be thrown in a noexcept context.
🧰 Tools
🪛 cppcheck (2.10-2)
[error] 191-191: Exception thrown in function declared not to throw exceptions.
(throwInNoexceptFunction)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
components/core/tests/test-clp_s-search.cpp (4)
49-58: 🛠️ Refactor suggestionAdd error handling for filesystem operations.
The path utility functions should handle potential filesystem errors to prevent runtime issues.
Apply this diff:
auto get_test_input_local_path() -> std::string { std::filesystem::path const current_file_path{__FILE__}; auto const tests_dir{current_file_path.parent_path()}; - return (tests_dir / get_test_input_path_relative_to_tests_dir()).string(); + auto const input_path = tests_dir / get_test_input_path_relative_to_tests_dir(); + if (!std::filesystem::exists(input_path)) { + throw std::runtime_error( + fmt::format("Test input file not found: {}", input_path.string())); + } + return input_path.string(); }
66-67: 🛠️ Refactor suggestionImprove directory creation error handling.
Directory creation should be wrapped in a try-catch block for proper error handling.
Apply this diff:
- std::filesystem::create_directory(cTestSearchArchiveDirectory); - REQUIRE((std::filesystem::is_directory(cTestSearchArchiveDirectory))); + try { + std::filesystem::create_directory(cTestSearchArchiveDirectory); + REQUIRE((std::filesystem::is_directory(cTestSearchArchiveDirectory))); + } catch (const std::filesystem::filesystem_error& e) { + throw std::runtime_error(fmt::format("Failed to create test directory: {}", e.what())); + }
90-108: 🛠️ Refactor suggestionImprove error handling and validation order.
- The error handling could be improved for better debugging.
- The size validation should be moved before the set comparison to fail fast.
Apply this diff:
void validate_results( std::vector<clp_s::search::VectorOutputHandler::QueryResult> const& results, std::vector<int64_t> const& expected_results ) { + REQUIRE(results.size() == expected_results.size()); std::set<int64_t> results_set; - bool results_are_valid_json = true; for (auto const& result : results) { try { auto json = nlohmann::json::parse(result.message); results_set.insert(json[cTestIdxKey].template get<int64_t>()); } catch (std::exception const& e) { - FAIL(fmt::format("Invalid JSON in result: {}", result.message)); + FAIL(fmt::format("Invalid JSON in result: {}. Error: {}", result.message, e.what())); return; } } std::set<int64_t> expected_results_set{expected_results.begin(), expected_results.end()}; REQUIRE(results_set == expected_results_set); - REQUIRE(results.size() == expected_results.size()); }
190-192:⚠️ Potential issueHandle potential exceptions in the test loop.
The static analysis tool correctly identifies potential exceptions in a noexcept context.
Apply this diff:
- for (auto const& [query, expected_results] : queries_and_results) { - search(query, false, expected_results); + for (auto const& [query, expected_results] : queries_and_results) { + try { + search(query, false, expected_results); + } catch (const std::exception& e) { + FAIL(fmt::format("Query '{}' failed: {}", query, e.what())); + } }🧰 Tools
🪛 cppcheck (2.10-2)
[error] 191-191: Exception thrown in function declared not to throw exceptions.
(throwInNoexceptFunction)
🧹 Nitpick comments (2)
components/core/CMakeLists.txt (1)
376-396: Consider adding documentation for the reducer component.The reducer component appears to be a new addition that handles record processing and network operations. Consider adding a README.md file to document its purpose, architecture, and usage.
components/core/tests/test-clp_s-search.cpp (1)
110-166: Improve resource management and nullptr checks.Consider these improvements:
- Use RAII for archive handling to ensure proper cleanup.
- Use more idiomatic nullptr checks.
Apply this diff:
- REQUIRE(nullptr != expr); + REQUIRE(expr != nullptr); - auto archive_reader = std::make_shared<clp_s::ArchiveReader>(); - // ... code ... - archive_reader->close(); + class ArchiveRAII { + public: + explicit ArchiveRAII(const clp_s::Path& path) + : reader_(std::make_shared<clp_s::ArchiveReader>()) { + reader_->open(path, clp_s::NetworkAuthOption{}); + } + ~ArchiveRAII() { reader_->close(); } + auto get() const { return reader_; } + private: + std::shared_ptr<clp_s::ArchiveReader> reader_; + }; + ArchiveRAII archive_raii(archive_path); + auto archive_reader = archive_raii.get();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
components/core/CMakeLists.txt(6 hunks)components/core/tests/LogSuppressor.hpp(1 hunks)components/core/tests/TestOutputCleaner.hpp(1 hunks)components/core/tests/test-clp_s-search.cpp(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- components/core/tests/TestOutputCleaner.hpp
- components/core/tests/LogSuppressor.hpp
🧰 Additional context used
📓 Path-based instructions (1)
components/core/tests/test-clp_s-search.cpp (1)
Pattern **/*.{cpp,hpp,java,js,jsx,ts,tsx}: - Prefer false == <expression> rather than !<expression>.
🪛 cppcheck (2.10-2)
components/core/tests/test-clp_s-search.cpp
[error] 191-191: Exception thrown in function declared not to throw exceptions.
(throwInNoexceptFunction)
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: ubuntu-jammy-static-linked-bins
- GitHub Check: ubuntu-jammy-dynamic-linked-bins
- GitHub Check: centos-stream-9-static-linked-bins
- GitHub Check: centos-stream-9-dynamic-linked-bins
- GitHub Check: ubuntu-focal-static-linked-bins
- GitHub Check: ubuntu-focal-dynamic-linked-bins
- GitHub Check: build-macos (macos-14, false)
- GitHub Check: build-macos (macos-13, false)
- GitHub Check: lint-check (ubuntu-latest)
- GitHub Check: lint-check (macos-latest)
- GitHub Check: build (macos-latest)
- GitHub Check: build-macos (macos-13, true)
🔇 Additional comments (8)
components/core/CMakeLists.txt (4)
Line range hint
302-349: LGTM: Search component files are well-organized.The search-related source files are logically grouped and follow a clear separation of concerns, with distinct files for different search operations like timestamp conditions, type narrowing, and output handling.
661-665: LGTM: Unit test target properly configured.The
unitTesttarget has been correctly updated to include both the new search and reducer source files.
625-625: Ensure test coverage for new components.New test files have been added for the search functionality. However, I don't see corresponding test files for the reducer component.
Also applies to: 630-630
538-539: Verify network utilities are not duplicated.The networking utilities appear in both the reducer component and the main CLP component. Let's verify there's no code duplication.
Also applies to: 376-396
✅ Verification successful
No code duplication found - proper code reuse implemented
The reducer component properly reuses the core networking utilities from CLP instead of duplicating them. The reducer's network utilities build higher-level operations on top of the base socket operations provided by CLP's socket_utils.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for potential code duplication in network utilities # Compare the content of both network utility files echo "Comparing network utility implementations:" diff <(cat src/clp/networking/socket_utils.cpp) <(cat src/reducer/network_utils.cpp) || true diff <(cat src/clp/networking/socket_utils.hpp) <(cat src/reducer/network_utils.hpp) || trueLength of output: 762
Script:
#!/bin/bash # Find all network utility files in the repository echo "Searching for network utility files:" fd -t f "network.*\.(cpp|hpp)$|socket.*\.(cpp|hpp)$" --exclude build --exclude dist # Show directory structure around networking files echo -e "\nDirectory structure around networking files:" fd -t d "network|socket" --exclude build --exclude distLength of output: 906
Script:
#!/bin/bash echo "Comparing network utility implementations:" echo -e "\nComparing CPP files:" diff components/core/src/clp/networking/socket_utils.cpp components/core/src/reducer/network_utils.cpp || true echo -e "\nComparing HPP files:" diff components/core/src/clp/networking/socket_utils.hpp components/core/src/reducer/network_utils.hpp || true echo -e "\nShowing contents of both implementations:" echo -e "\n=== socket_utils.cpp ===" cat components/core/src/clp/networking/socket_utils.cpp echo -e "\n=== network_utils.cpp ===" cat components/core/src/reducer/network_utils.cppLength of output: 13150
components/core/tests/test-clp_s-search.cpp (4)
1-37: LGTM! Well-organized includes and constants.The includes are properly organized and the constants follow good naming conventions.
38-48: LGTM! Clear function declarations following modern C++ conventions.Good use of anonymous namespace and trailing return type syntax.
87-87: LGTM! Proper boolean comparison style.Good use of
false ==comparison style as per coding guidelines.
169-189: LGTM! Well-structured test cases with good coverage.The test case uses Catch2's GENERATE effectively for combinatorial testing of different configurations.
| {"idx": 0, "a": "clp string"} | ||
| {"idx": 1, "msg": "Msg 1: \"Abc123\""} | ||
| {"idx": 2, "msg": "Msg 2: 'Abc123'"} | ||
| {"idx": 3, "msg": "Msg 3: \nAbc123"} | ||
| {"idx": 4, "skip_msg": "Msg 4: \\Abc123"} | ||
| {"idx": 5, "msg": "Msg 5: \rAbc123"} | ||
| {"idx": 6, "msg": "Msg 6: \tAbc123"} | ||
| {"idx": 7, "arr": [{"a": 999}, {"b": 1001}]} | ||
| {"idx": 8, "arr": {"a": 999, "b": 1001}} |
There was a problem hiding this comment.
Do we want to include other data types (e.g. boolean, float) in the future?
There was a problem hiding this comment.
Yeah, just wanted to get all the boilerplate in + some key edge cases so that we have basic coverage and its easy to add more later.
Specifically I'll probably add a lot more cases to make sure I don't introduce regressions when I work on solving #588 since that fix will likely involve some tricky AST/ConvertToExists/Schema matching changes.
| // Delete copy & move constructors and assignment operators | ||
| LogSuppressor(LogSuppressor const& other) = delete; | ||
| LogSuppressor(LogSuppressor&& other) = delete; | ||
| auto operator=(LogSuppressor const& other) -> LogSuppressor& = delete; | ||
| auto operator=(LogSuppressor&& other) -> LogSuppressor& = delete; | ||
|
|
There was a problem hiding this comment.
I'm curious why we need to turn off logging for test cases and why it's only used in test-kql and test-sql.
There was a problem hiding this comment.
We have some test cases that ensure illegal kql expressions fail to parse (and will do the same for sql). Inside of our KQL parser we log whenever we encounter a parsing error. We turn off logging just so that running those tests doesn't fill the terminal with error messages about failed parsing.
| {R"aa(msg: "Msg 1: \"Abc123\"")aa", {1}}, | ||
| {R"aa(msg: "Msg 2: 'Abc123'")aa", {2}}, | ||
| {R"aa(msg: "Msg 3: \nAbc123")aa", {3}}, | ||
| // CLP incorrectly generates no subqueries in Grep::process_raw_query for the following | ||
| // query, so we skip it for now. | ||
| F438 | //{R"aa(msg: "Msg 4: \\Abc123")aa", {4}} | |
| {R"aa(msg: "Msg 5: \rAbc123")aa", {5}}, | ||
| {R"aa(msg: "Msg 6: \tAbc123")aa", {6}}, | ||
| {R"aa(msg: "*Abc123*")aa", {1, 2, 3, 5, 6}}, |
There was a problem hiding this comment.
I guess we'll also include more query shapes (e.g. AND, ?) in the future?
There was a problem hiding this comment.
Yep, just trying to get the boilerplate and basic coverage in for now. We can add more exhaustive tests as needed over time.
Description
This PR adds end to end search tests for clp-s and associated boilerplate with initial tests inspired by #590 and #254. More tests can be added in the future to expand coverage.
This PR also touches test-clp_s-end-to-end in order to test single-file-archive support.
The basic flow for the search tests is:
Validation performed
Summary by CodeRabbit
Release Notes
New Features
Testing
clp_ssearch functionality.Performance
Infrastructure