Skip to content

compiletest: known-bug / crashes: allow for an "auxiliary" directory to contain files that do not have a "known-bug" directive#133013

Merged
bors merged 1 commit intorust-lang:masterfrom
matthiaskrgr:crash_aux
Nov 14, 2024
Merged

compiletest: known-bug / crashes: allow for an "auxiliary" directory to contain files that do not have a "known-bug" directive#133013
bors merged 1 commit intorust-lang:masterfrom
matthiaskrgr:crash_aux

Conversation

@matthiaskrgr
Copy link
Member

@matthiaskrgr matthiaskrgr commented Nov 13, 2024

Fixes #133009

r? @jieyouxu

@matthiaskrgr
compiletest: known-bug / crashes: allow for an "auxiliary" directory …
73c6494 
…to contain files that do not have a "known-bug" directive

Fixes rust-lang#133009
@rustbot
Copy link
Collaborator

rustbot commented Nov 13, 2024

Could not assign reviewer from: jieyouxu.
User(s) jieyouxu are either the PR author, already assigned, or on vacation, and there are no other candidates.
Use r? to specify someone else to assign.

@rustbot
Copy link
Collaborator

rustbot commented Nov 13, 2024

r? @onur-ozkan

rustbot has assigned @onur-ozkan.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Nov 13, 2024
@jieyouxu
Copy link
Member

jieyouxu commented Nov 13, 2024 •
edited
Loading

FWIW in ui tests the aux build logic supports setup like

  • tests/ui/foo/auxiliary/bark.rs
  • tests/ui/foo/meow.rs

i.e. it's not a hard coded fixed giga aux directory.

But if crashes don't support nested auxiliary I guess this logic makes sense for tidy?

@matthiaskrgr
Copy link
Member Author

matthiaskrgr commented Nov 13, 2024

I never really planned for the crashes directory to have subdirs .. 😅

@rust-log-analyzer
Copy link
Collaborator

rust-log-analyzer commented Nov 13, 2024

The job mingw-check-tidy failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot) 

COPY host-x86_64/mingw-check/validate-toolstate.sh /scripts/
COPY host-x86_64/mingw-check/validate-error-codes.sh /scripts/

# NOTE: intentionally uses python2 for x.py so we can test it still works.
# validate-toolstate only runs in our CI, so it's ok for it to only support python3.
ENV SCRIPT TIDY_PRINT_DIFF=1 python2.7 ../x.py test \
           --stage 0 src/tools/tidy tidyselftest --extra-checks=py:lint,cpp:fmt
# This file is autogenerated by pip-compile with Python 3.10
# by the following command:
#
#    pip-compile --allow-unsafe --generate-hashes reuse-requirements.in
---
#13 2.931 Building wheels for collected packages: reuse
#13 2.932   Building wheel for reuse (pyproject.toml): started
#13 3.182   Building wheel for reuse (pyproject.toml): finished with status 'done'
#13 3.184   Created wheel for reuse: filename=reuse-4.0.3-cp310-cp310-manylinux_2_35_x86_64.whl size=132720 sha256=026f3bb0f1aa8090b861fd0a0939cb1a782396d84c8aab7875096557d637a0f6
#13 3.184   Stored in directory: /tmp/pip-ephem-wheel-cache-216v23xj/wheels/3d/8d/0a/e0fc6aba4494b28a967ab5eaf951c121d9c677958714e34532
#13 3.186 Installing collected packages: boolean-py, binaryornot, tomlkit, reuse, python-debian, markupsafe, license-expression, jinja2, chardet, attrs
#13 3.588 Successfully installed attrs-23.2.0 binaryornot-0.4.4 boolean-py-4.0 chardet-5.2.0 jinja2-3.1.4 license-expression-30.3.0 markupsafe-2.1.5 python-debian-0.1.49 reuse-4.0.3 tomlkit-0.13.0
#13 3.588 WARNING: Running pip as the 'root' user can result in broken permissions and conflicting behaviour with the system package manager. It is recommended to use a virtual environment instead: https://pip.pypa.io/warnings/venv
#13 4.141 Collecting virtualenv
#13 4.141 Collecting virtualenv
#13 4.212   Downloading virtualenv-20.27.1-py3-none-any.whl (3.1 MB)
#13 4.310      ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 3.1/3.1 MB 32.8 MB/s eta 0:00:00
#13 4.368 Collecting filelock<4,>=3.12.2
#13 4.382   Downloading filelock-3.16.1-py3-none-any.whl (16 kB)
#13 4.401 Collecting distlib<1,>=0.3.7
#13 4.416   Downloading distlib-0.3.9-py2.py3-none-any.whl (468 kB)
#13 4.424      ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 469.0/469.0 KB 76.6 MB/s eta 0:00:00
#13 4.457 Collecting platformdirs<5,>=3.9.1
#13 4.472   Downloading platformdirs-4.3.6-py3-none-any.whl (18 kB)
#13 4.554 Installing collected packages: distlib, platformdirs, filelock, virtualenv
#13 4.732 Successfully installed distlib-0.3.9 filelock-3.16.1 platformdirs-4.3.6 virtualenv-20.27.1
#13 DONE 4.8s

#14 [7/8] COPY host-x86_64/mingw-check/validate-toolstate.sh /scripts/
#14 DONE 0.0s
---
DirectMap4k:      298944 kB
DirectMap2M:     7041024 kB
DirectMap1G:    11534336 kB
##[endgroup]
Executing TIDY_PRINT_DIFF=1 python2.7 ../x.py test            --stage 0 src/tools/tidy tidyselftest --extra-checks=py:lint,cpp:fmt
+ TIDY_PRINT_DIFF=1 python2.7 ../x.py test --stage 0 src/tools/tidy tidyselftest --extra-checks=py:lint,cpp:fmt
    Finished `dev` profile [unoptimized] target(s) in 0.04s
##[endgroup]
WARNING: `rust.download-rustc` is enabled. The `rust.channel` option will be overridden by the CI rustc's channel.
downloading https://static.rust-lang.org/dist/2024-10-16/rustfmt-nightly-x86_64-unknown-linux-gnu.tar.xz
---
fmt check
fmt: checked 5661 files
tidy check
tidy: Skipping binary file check, read-only filesystem
removing old virtual environment
creating virtual environment at '/checkout/obj/build/venv' using 'python3.10'
Requirement already satisfied: pip in ./build/venv/lib/python3.10/site-packages (24.3.1)
ERROR: Could not install packages due to an OSError: HTTPSConnectionPool(host='files.pythonhosted.org', port=443): Max retries exceeded with url: /packages/e0/7d/7f8df0fdbbbefc4362d3eca6b69b7a8a4249a8a88dabc00a207d31fddcd7/black-24.4.2-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (Caused by ResponseError('too many 502 error responses'))
some tidy checks failed
tidy error: failed to install requirements at /checkout/src/tools/tidy/config/requirements.txt
tidy error: failed to install requirements at /checkout/src/tools/tidy/config/requirements.txt
Command has failed. Rerun with -v to see more details.
  local time: Wed Nov 13 21:50:05 UTC 2024
  network time: Wed, 13 Nov 2024 21:50:05 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

@BoxyUwU BoxyUwU mentioned this pull request Nov 14, 2024
Merged
@onur-ozkan
Copy link
Contributor

onur-ozkan commented Nov 14, 2024

@bors r+

@bors
Copy link
Collaborator

bors commented Nov 14, 2024

📌 Commit 73c6494 has been approved by onur-ozkan

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 14, 2024
jieyouxu
jieyouxu reviewed Nov 14, 2024
View reviewed changes
src/tools/tidy/src/known_bug.rs
Comment on lines +11 to +22
// files in "auxiliary" do not need to crash by themselves
let test_path_segments =
file.iter().map(|s| s.to_string_lossy().into()).collect::<Vec<String>>();
let test_path_segments_str =
test_path_segments.iter().map(|s| s.as_str()).collect::<Vec<&str>>();

if !matches!(test_path_segments_str[..], [
..,
"tests",
"crashes",
"auxiliary",
_aux_file_rs
Copy link
Member

@jieyouxu jieyouxu Nov 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: I think the aux test file check be simplified to something like

let crashes_aux_dir_prefix = test_root.join("crashes").join("auxiliary"); // probably  pass this as a ref

let is_crashes_aux_file = file.starts_with(crashes_aux_dir_prefix);

If we don't support arbitrary nested auxiliary folders in tests/crashes.

Collecting into Vec<String> in test_path_segments would do a bunch of small string allocations, right?

@GuillaumeGomez GuillaumeGomez mentioned this pull request Nov 14, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 14, 2024
@bors
Auto merge of rust-lang#133032 - GuillaumeGomez:rollup-vqakdmw, r=Gui…
a4cedec 
…llaumeGomez

Rollup of 5 pull requests

Successful merges:

 - rust-lang#132010 (ci: Enable full `debuginfo-level=2` in `DEPLOY_ALT`)
 - rust-lang#132310 (compiletest: add `max-llvm-major-version` directive)
 - rust-lang#132773 (PassWrapper: disable UseOdrIndicator for Asan Win32)
 - rust-lang#133013 (compiletest: known-bug / crashes: allow for an "auxiliary" directory to contain files that do not have a "known-bug" directive)
 - rust-lang#133027 (Fix a copy-paste issue in the NuttX raw type definition)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 6a783a4 into rust-lang:master Nov 14, 2024
@rustbot rustbot added this to the 1.84.0 milestone Nov 14, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 14, 2024
@rust-timer
Unrolled build for rust-lang#133013
672532e 
Rollup merge of rust-lang#133013 - matthiaskrgr:crash_aux, r=onur-ozkan

compiletest: known-bug / crashes: allow for an "auxiliary" directory to contain files that do not have a "known-bug" directive

Fixes rust-lang#133009

r? `@jieyouxu`
@matthiaskrgr matthiaskrgr deleted the crash_aux branch January 25, 2025 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jieyouxu jieyouxu jieyouxu left review comments

Assignees

@onur-ozkan onur-ozkan

Labels

A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Projects

None yet

Milestone

1.84.0

Development

Successfully merging this pull request may close these issues.

aux-build tests/crashes tests don't seem to work

6 participants

@matthiaskrgr @rustbot @jieyouxu @rust-log-analyzer @onur-ozkan @bors