mirror of
https://github.com/openai/codex.git
synced 2026-09-11 20:36:49 +00:00
fix: address coverage gap in argument-comment-lint
This commit is contained in:
3
.github/workflows/rust-ci-full.yml
vendored
3
.github/workflows/rust-ci-full.yml
vendored
@@ -116,6 +116,7 @@ jobs:
|
||||
BUILDBUDDY_API_KEY: ${{ secrets.BUILDBUDDY_API_KEY }}
|
||||
shell: bash
|
||||
run: |
|
||||
bazel_targets="$(./tools/argument-comment-lint/list-bazel-targets.sh)"
|
||||
./.github/scripts/run-bazel-ci.sh \
|
||||
-- \
|
||||
build \
|
||||
@@ -123,7 +124,7 @@ jobs:
|
||||
--keep_going \
|
||||
--build_metadata=COMMIT_SHA=${GITHUB_SHA} \
|
||||
-- \
|
||||
//codex-rs/...
|
||||
${bazel_targets}
|
||||
- name: Run argument comment lint on codex-rs via packaged wrapper
|
||||
if: ${{ runner.os == 'Windows' }}
|
||||
shell: bash
|
||||
|
||||
3
.github/workflows/rust-ci.yml
vendored
3
.github/workflows/rust-ci.yml
vendored
@@ -176,6 +176,7 @@ jobs:
|
||||
BUILDBUDDY_API_KEY: ${{ secrets.BUILDBUDDY_API_KEY }}
|
||||
shell: bash
|
||||
run: |
|
||||
bazel_targets="$(./tools/argument-comment-lint/list-bazel-targets.sh)"
|
||||
./.github/scripts/run-bazel-ci.sh \
|
||||
-- \
|
||||
build \
|
||||
@@ -183,7 +184,7 @@ jobs:
|
||||
--keep_going \
|
||||
--build_metadata=COMMIT_SHA=${GITHUB_SHA} \
|
||||
-- \
|
||||
//codex-rs/...
|
||||
${bazel_targets}
|
||||
- name: Run argument comment lint on codex-rs via packaged wrapper
|
||||
if: ${{ runner.os == 'Windows' }}
|
||||
shell: bash
|
||||
|
||||
@@ -313,9 +313,15 @@ async fn timed_out_snapshot_shell_is_terminated() -> Result<()> {
|
||||
shell_snapshot: crate::shell::empty_shell_snapshot_receiver(),
|
||||
};
|
||||
|
||||
let err = run_script_with_timeout(&shell, &script, Duration::from_secs(1), true, dir.path())
|
||||
.await
|
||||
.expect_err("snapshot shell should time out");
|
||||
let err = run_script_with_timeout(
|
||||
&shell,
|
||||
&script,
|
||||
Duration::from_secs(1),
|
||||
/*use_login_shell*/ true,
|
||||
dir.path(),
|
||||
)
|
||||
.await
|
||||
.expect_err("snapshot shell should time out");
|
||||
assert!(
|
||||
err.to_string().contains("timed out"),
|
||||
"expected timeout error, got {err:?}"
|
||||
|
||||
@@ -274,7 +274,10 @@ mod tests {
|
||||
#[test]
|
||||
fn managed_network_enforces_seccomp_even_for_full_network_policy() {
|
||||
assert_eq!(
|
||||
should_install_network_seccomp(NetworkSandboxPolicy::Enabled, true),
|
||||
should_install_network_seccomp(
|
||||
NetworkSandboxPolicy::Enabled,
|
||||
/*allow_network_for_proxy*/ true
|
||||
),
|
||||
true
|
||||
);
|
||||
}
|
||||
@@ -282,7 +285,10 @@ mod tests {
|
||||
#[test]
|
||||
fn full_network_policy_without_managed_network_skips_seccomp() {
|
||||
assert_eq!(
|
||||
should_install_network_seccomp(NetworkSandboxPolicy::Enabled, false),
|
||||
should_install_network_seccomp(
|
||||
NetworkSandboxPolicy::Enabled,
|
||||
/*allow_network_for_proxy*/ false
|
||||
),
|
||||
false
|
||||
);
|
||||
}
|
||||
@@ -291,18 +297,22 @@ mod tests {
|
||||
fn restricted_network_policy_always_installs_seccomp() {
|
||||
assert!(should_install_network_seccomp(
|
||||
NetworkSandboxPolicy::Restricted,
|
||||
false
|
||||
/*allow_network_for_proxy*/ false
|
||||
));
|
||||
assert!(should_install_network_seccomp(
|
||||
NetworkSandboxPolicy::Restricted,
|
||||
true
|
||||
/*allow_network_for_proxy*/ true
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn managed_proxy_routes_use_proxy_routed_seccomp_mode() {
|
||||
assert_eq!(
|
||||
network_seccomp_mode(NetworkSandboxPolicy::Enabled, true, true),
|
||||
network_seccomp_mode(
|
||||
NetworkSandboxPolicy::Enabled,
|
||||
/*allow_network_for_proxy*/ true,
|
||||
/*proxy_routed_network*/ true
|
||||
),
|
||||
Some(NetworkSeccompMode::ProxyRouted)
|
||||
);
|
||||
}
|
||||
@@ -310,7 +320,11 @@ mod tests {
|
||||
#[test]
|
||||
fn restricted_network_without_proxy_routing_uses_restricted_mode() {
|
||||
assert_eq!(
|
||||
network_seccomp_mode(NetworkSandboxPolicy::Restricted, false, false),
|
||||
network_seccomp_mode(
|
||||
NetworkSandboxPolicy::Restricted,
|
||||
/*allow_network_for_proxy*/ false,
|
||||
/*proxy_routed_network*/ false
|
||||
),
|
||||
Some(NetworkSeccompMode::Restricted)
|
||||
);
|
||||
}
|
||||
@@ -318,7 +332,11 @@ mod tests {
|
||||
#[test]
|
||||
fn full_network_without_managed_proxy_skips_network_seccomp_mode() {
|
||||
assert_eq!(
|
||||
network_seccomp_mode(NetworkSandboxPolicy::Enabled, false, false),
|
||||
network_seccomp_mode(
|
||||
NetworkSandboxPolicy::Enabled,
|
||||
/*allow_network_for_proxy*/ false,
|
||||
/*proxy_routed_network*/ false
|
||||
),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
@@ -718,7 +718,8 @@ mod tests {
|
||||
#[test]
|
||||
fn rewrites_proxy_url_to_local_loopback_port() {
|
||||
let rewritten =
|
||||
rewrite_proxy_env_value("socks5h://127.0.0.1:8081", 43210).expect("rewritten value");
|
||||
rewrite_proxy_env_value("socks5h://127.0.0.1:8081", /*local_port*/ 43210)
|
||||
.expect("rewritten value");
|
||||
assert_eq!(rewritten, "socks5h://127.0.0.1:43210");
|
||||
}
|
||||
|
||||
|
||||
4
justfile
4
justfile
@@ -74,7 +74,7 @@ bazel-clippy:
|
||||
|
||||
[no-cd]
|
||||
bazel-argument-comment-lint:
|
||||
bazel build --config=argument-comment-lint -- //codex-rs/...
|
||||
bazel build --config=argument-comment-lint -- $(./tools/argument-comment-lint/list-bazel-targets.sh)
|
||||
|
||||
bazel-remote-test:
|
||||
bazel test --test_tag_filters=-argument-comment-lint //... --config=remote --platforms=//:rbe --keep_going
|
||||
@@ -102,7 +102,7 @@ write-hooks-schema:
|
||||
[no-cd]
|
||||
argument-comment-lint *args:
|
||||
if [ "$#" -eq 0 ]; then \
|
||||
bazel build --config=argument-comment-lint -- //codex-rs/...; \
|
||||
bazel build --config=argument-comment-lint -- $(./tools/argument-comment-lint/list-bazel-targets.sh); \
|
||||
else \
|
||||
./tools/argument-comment-lint/run-prebuilt-linter.py "$@"; \
|
||||
fi
|
||||
|
||||
@@ -129,7 +129,8 @@ Run the lint against `codex-rs` from the repo root:
|
||||
|
||||
```bash
|
||||
just argument-comment-lint
|
||||
bazel build --config=argument-comment-lint -- //codex-rs/...
|
||||
bazel build --config=argument-comment-lint -- \
|
||||
$(./tools/argument-comment-lint/list-bazel-targets.sh)
|
||||
./tools/argument-comment-lint/run-prebuilt-linter.py -p codex-core
|
||||
just argument-comment-lint -p codex-core
|
||||
```
|
||||
@@ -138,7 +139,10 @@ If no package selection is provided, `just argument-comment-lint` now defaults
|
||||
to the Bazel aspect path over `//codex-rs/...`. The Python wrappers remain the
|
||||
package-scoped escape hatch and still default the underlying Cargo invocation
|
||||
to `--all-targets` unless you explicitly narrow the target set, so targeted
|
||||
wrapper runs cover test-only call sites by default.
|
||||
wrapper runs cover test-only call sites by default. The Bazel entrypoints use
|
||||
`tools/argument-comment-lint/list-bazel-targets.sh` to add the internal
|
||||
manual `*-unit-tests-bin` Rust targets explicitly, so inline `#[cfg(test)]`
|
||||
call sites are covered without pulling in unrelated manual release targets.
|
||||
|
||||
Repo runs also promote `uncommented_anonymous_literal_argument` to an error by
|
||||
default:
|
||||
|
||||
13
tools/argument-comment-lint/list-bazel-targets.sh
Executable file
13
tools/argument-comment-lint/list-bazel-targets.sh
Executable file
@@ -0,0 +1,13 @@
|
||||
#!/usr/bin/env bash
|
||||
|
||||
set -euo pipefail
|
||||
|
||||
repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)"
|
||||
cd "${repo_root}"
|
||||
|
||||
# Bazel wildcard builds skip manual targets, which misses the internal
|
||||
# `*-unit-tests-bin` rust_test targets generated by `codex_rust_crate()`.
|
||||
# Add only those manual rust_test targets explicitly so inline `#[cfg(test)]`
|
||||
# call sites are linted without pulling in unrelated manual release targets.
|
||||
printf '%s\n' "//codex-rs/..."
|
||||
bazel query 'kind("rust_test rule", attr(tags, "manual", //codex-rs/...))'
|
||||
Reference in New Issue
Block a user