Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test_generate_json_schema failing as of 0.9.2 #15707

Closed
WhyNotHugo opened this issue Jan 24, 2025 · 6 comments · Fixed by #15627
Closed

test_generate_json_schema failing as of 0.9.2 #15707

WhyNotHugo opened this issue Jan 24, 2025 · 6 comments · Fixed by #15627
Assignees
Labels
testing Related to testing Ruff itself

Comments

@WhyNotHugo
Copy link
Contributor

WhyNotHugo commented Jan 24, 2025

Description

As of 0.9.2, the test_generate_json_schema test fails when building from source. Of note, we build with the default features.

Full build output is here: https://paste.sr.ht/~whynothugo/41004049e3a4f9d2098bec4593a68fd949bd5cd4

The output for this specific test starts at line 1816 and is 4251 lines long.

This error is produced when building the Alpine packages. The build steps can be summarised as:

# prepare()
# shadow git repo for tests
git init -q

# Avoid downloading a different toolchain on systems with rustup installed.
rm rust-toolchain.toml

cargo fetch --locked

# build()
gpep517 build-wheel \
        --wheel-dir .dist \
        --config-json '{"build-args": "--frozen"}' \
        --output-fd 3 3>&1 >&2

./target/release/ruff generate-shell-completion bash > $pkgname.bash
./target/release/ruff generate-shell-completion fish > $pkgname.fish
./target/release/ruff generate-shell-completion zsh > $pkgname.zsh

# Update ruff.schema.json as the pre-built one is generated
# using the '--all-features' Cargo flag which we don't pass.
cargo dev generate-all

# check()
unset CI_PROJECT_DIR
cargo test --frozen --workspace --exclude ruff_benchmark
@InSyncWithFoo
Copy link
Contributor

InSyncWithFoo commented Jan 24, 2025

Line 5474 is the one in question:

5472 |          "PLW01",
5473 |          "PLW010",
5474 | �[32m>        "PLW0101",�[0m
5475 |          "PLW0108",
5476 |          "PLW012",

PLW0101 was added in #10801. Its implementation turned out to have an infinite loop bug, which was reported in #15248. #15252 then made it a test-only rule, adding these attributes:

#[cfg(any(feature = "test-rules", test))]  // < !!!
if checker.enabled(Rule::UnreachableCode) {
    /* ... */
}

#15278 fixed the issue, but did not revert the attributes.

@dhruvmanila
Copy link
Member

This will be fixed with #15627

@dhruvmanila dhruvmanila self-assigned this Jan 24, 2025
@dhruvmanila dhruvmanila added the testing Related to testing Ruff itself label Jan 24, 2025
@InSyncWithFoo
Copy link
Contributor

@dhruvmanila I don't think so. #15278 fixed PLW0101, so the correct thing to do is perhaps to repromote it.

@dhruvmanila
Copy link
Member

I don't think we're sure of promoting it yet, so for now we should just fix the JSON schema generation. cc @dylwil3 who's taking the lead on this specific issue.

@InSyncWithFoo
Copy link
Contributor

Now that I rechecked it, @dylwil3 did say that the rule should be kept in test-only realm:

Added the latest example from the fuzzer, which passes. So let's merge this in and continue to test this rule and improve the CFG implementation.

It has been two weeks since then, though, and there don't seem to be any new issues. Maybe it's time?

@WhyNotHugo
Copy link
Contributor Author

Thanks for the quick fix!

algitbot pushed a commit to alpinelinux/aports that referenced this issue Jan 24, 2025
Includes a patch with a recent commit from upstream to fix a failing test.

See: astral-sh/ruff#15707
Closes: https://gitlab.alpinelinux.org/alpine/aports/-/merge_requests/78284
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Related to testing Ruff itself
Projects
None yet
3 participants