github-actions[bot] commented on code in PR #68607:
URL: https://github.com/apache/doris/pull/68607#discussion_r4366635657
##########
be/test/storage/index/inverted/snii/snii_golden_corpus_test.cpp:
##########
@@ -454,16 +461,46 @@ Observation observe(const Sample& sample, const
OpenedSample& opened, const Quer
// ---------------------------------------------------------------- fixture
+std::filesystem::path corpus_directory(std::string_view explicit_dir,
std::string_view doris_home,
+ const std::filesystem::path&
source_file) {
+ // Explicit directories select an external corpus or a write destination.
+ if (!explicit_dir.empty()) return std::filesystem::path(explicit_dir);
+ // The runner starts at DORIS_HOME, where build-relative paths such as
../test miss the corpus.
+ const auto source_dir = doris_home.empty() ? source_file.parent_path()
+ :
std::filesystem::path(doris_home) /
Review Comment:
[P1] Resolve the bundled corpus from the runner's source directory
`run-be-ut.sh` overwrites `DORIS_HOME` with `${CMAKE_BUILD_DIR}/test/`
before launching GTest (line 563), so this branch looks under
`<build>/test/be/test/.../compatibility_corpus`, where none of the committed
files are copied. Without `SNII_GOLDEN_DIR`, `SniiGoldenCorpus.WriteOrVerify`
therefore reports eight missing expectation files and checks zero observations
in an ordinary BE unit-test run. The new path tests use a source-root
`DORIS_HOME` and miss this case. Please use a stable source-directory anchor
(or explicitly copy the fixtures) and cover the actual runner environment.
##########
be/test/storage/index/inverted/snii/testdata/compatibility_corpus/english_docs.expect:
##########
@@ -0,0 +1,44 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+# sample=english_docs doc_count=20 lower_case=true parser=english
+5 hello 0 0,10,18,19 0 4,5,6,11
+5 hello%20world 0 0,10,18,19 0 4,5,6,11
+5 nonexistent 0 0 4,5,6,11
+5 the 0 2 0 4,5,6,11
+5 重复 0 0 4,5,6,11
+5 %1fhidden 0 12 0 4,5,6,11
+5 prefix 0 14 0 4,5,6,11
+5 tok 0 16 0 4,5,6,11
+5 0 0 4,5,6,11
+6 hello%20world 0 0,10,19 0 4,5,6,11
+6 again%20and 0 10 0 4,5,6,11
+6 alpha%20beta%20gamma 0 15 0 4,5,6,11
+6 hello%20nonexistent 0 0 4,5,6,11
+7 hello%20world -6001 0 4,5,6,11
Review Comment:
[P1] Reconcile the docs-only phrase error codes
With `SNII_GOLDEN_DIR` pointing at the committed corpus,
`MATCH_PHRASE("hello world")` reaches `SniiIndexSource::open_terms` with
`positions=true` and returns `Status::NotSupported`, whose code is
`NOT_IMPLEMENTED_ERROR` (`3`), rather than `-6001`. The shared phrase engine's
exception boundary preserves that code. The same mismatch occurs in nine
phrase/prefix records here (lines 31-34, 36, and 38-41), so the newly mandatory
test still reports 27 mismatches across its three cache modes after discovery
is corrected. Please reconcile all nine status expectations with the intended
current reader contract.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
