test: Migrate test_semanticdb.sh to Bazel-native sh_test targets - #1949
test: Migrate test_semanticdb.sh to Bazel-native sh_test targets#1949bor-p-s wants to merge 4 commits into
Conversation
| # Keep the nested build's convenience symlinks out of the workspace so they do | ||
| # not clobber the parent invocation's `bazel-bin` etc. | ||
| _nested_bazel_common_opts+=("--symlink_prefix=${_nested_bazel_output_base}/convenience_symlinks/") | ||
| # not clobber the parent invocation's `bazel-bin` etc. Kept separate from |
There was a problem hiding this comment.
Oh, so up to now there was no bazel query run via nested bazel?
I remember you've used cquery instead of query (probably for this reason) somewhere recently right?
There was a problem hiding this comment.
Right, switched to cquery — it already accepts --symlink_prefix and supports labels(srcs, ...). Reverted the query-specific branching in nested_bazel_run, so this file's diff is now just the labels( addition to MSYS2_ARG_CONV_EXCL (still needed since the label sits mid-string here too).
There was a problem hiding this comment.
Up to you - query + fix to nested_bazel.sh can also stay here and in other places (cquery can arguably be confusing as we don't care about configured graph here)
There was a problem hiding this comment.
Fair point on cquery being confusing here — switched back to query + the --symlink_prefix skip, since we really just need srcs membership, not the configured graph.
| [ | ||
| sh_test( | ||
| name = "produces_semanticdb_test_scala%d_%s" % ( | ||
| scala_majver, |
There was a problem hiding this comment.
Nit: IIRC the scheme calls these segments EPOCH.MAJOR.MINOR, pls verify & if so consider changing to scala_epoch (unless this repo uses major for that already)
There was a problem hiding this comment.
Confirmed — extract_major_version already returns "2.13"/"3.3" here, so scala_majver collided with that. Renamed to scala_epoch.
| fi | ||
|
|
||
| if [[ "$(find "${target_out}" -type f -name '*.semanticdb' | wc -l)" -gt 0 ]]; then | ||
| echo "Error: Semanticdb files erroneously found in target output" >&2 |
There was a problem hiding this comment.
Oh, so it shouldn't be a loose file but inside a jar instead? if so, pls clarify the error message
There was a problem hiding this comment.
Right, it's loose files, the jar check is separate below. Reworded both messages to say which and name the file.
| source "${bazel_bin}/test/semanticdb/semantic_provider_vars_all.sh" | ||
|
|
||
| if [[ "${semanticdb_enabled}" -ne 1 ]]; then | ||
| echo "Error: SemanticdbInfo.semanticdb_enabled not equal to true" >&2 |
There was a problem hiding this comment.
It's not clear from error message what kind of config (?) key it is and where it's read from
There was a problem hiding this comment.
(same for the ones below)
There was a problem hiding this comment.
Reworded to show actual vs expected, plus one comment noting these are SemanticdbInfo fields from scala/semanticdb_provider.bzl.
There was a problem hiding this comment.
Same fix applied to all four checks.
2 participants
Description
The other 4 functions check post-build state: provider values read from a generated script, whether
.semanticdboutput was produced, and whether a source glob picks up an added/removed file. That's outside whatexpect_build_failure.bzl's macros model (a build/test pass/fail outcome), so these are hand-writtensh_testscripts onnested_bazel.sh, the same pattern asexpect_no_ijar.sh.nested_bazel.shgets two small fixes for the newbazel querycall it needs: it now skips--symlink_prefixforqueryspecifically, since that's the one subcommand that rejects the flag, and its Windows arg-conversion list now coverslabels(too, the same label-mangling guard it already has for--extra_toolchains=. This second fix is reasoned from the existing guard, not verified on a real Windows run.scala/BUILDandscala/private/BUILDeach gain oneexports_filesentry (semanticdb_provider.bzl,phases/phase_semanticdb.bzl), so the Scala 2produces_semanticdb_testvariants can list them asdata. Their fixture's only own action is a template expansion, which the fingerprint mechanism reads by mnemonic only, so a bug in the provider's fields wouldn't otherwise bust the test's cache.Motivation
Continues the move off the old shell-test runner (
test_rules_scala.sh) toward nativebazel test //....Release impact: tests only.