Skip to content

Fix integration tests #2 #11225

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jul 26, 2023
+18 −14
Merged

Fix integration tests #2 #11225

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c3079a9
Fix integration tests
flip1995 Jun 17, 2023
12b63f5
ci: integration tests: print clippy run output and set LD_LIBRARY_PAT…
matthiaskrgr Jul 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 14 additions & 14 deletions .github/workflows/clippy_bors.yml
View file
Original file line number Diff line number Diff line change
@@ -187,16 +187,14 @@ jobs:
- name: Extract Binaries
run: |
DIR=$CARGO_TARGET_DIR/debug
rm $DIR/deps/integration-*.d
mv $DIR/deps/integration-* $DIR/integration
find $DIR/deps/integration-* -executable ! -type d | xargs -I {} mv {} $DIR/integration
find $DIR ! -executable -o -type d ! -path $DIR | xargs rm -rf
rm -rf $CARGO_TARGET_DIR/release
- name: Upload Binaries
uses: actions/upload-artifact@v1
uses: actions/upload-artifact@v3
with:
name: target
path: target
name: binaries
path: target/debug

integration:
needs: integration_build
@@ -206,16 +204,13 @@ jobs:
matrix:
integration:
- 'rust-lang/cargo'
# FIXME: re-enable once fmt_macros is renamed in RLS
# - 'rust-lang/rls'
- 'rust-lang/chalk'
- 'rust-lang/rustfmt'
- 'Marwes/combine'
- 'Geal/nom'
- 'rust-lang/stdarch'
- 'serde-rs/serde'
# FIXME: chrono currently cannot be compiled with `--all-targets`
# - 'chronotope/chrono'
- 'chronotope/chrono'
- 'hyperium/hyper'
- 'rust-random/rand'
- 'rust-lang/futures-rs'
@@ -237,12 +232,17 @@ jobs:
- name: Install toolchain
run: rustup show active-toolchain

- name: Set LD_LIBRARY_PATH
run: |
SYSROOT=$(rustc --print sysroot)
echo "LD_LIBRARY_PATH=${SYSROOT}/lib${LD_LIBRARY_PATH+:${LD_LIBRARY_PATH}}" >> $GITHUB_ENV
# Download
- name: Download target dir
uses: actions/download-artifact@v1
uses: actions/download-artifact@v3
with:
name: target
path: target
name: binaries
path: target/debug

- name: Make Binaries Executable
run: chmod +x $CARGO_TARGET_DIR/debug/*
@@ -251,7 +251,7 @@ jobs:
- name: Test ${{ matrix.integration }}
run: |
RUSTUP_TOOLCHAIN="$(rustup show active-toolchain | grep -o -E "nightly-[0-9]{4}-[0-9]{2}-[0-9]{2}")" \
$CARGO_TARGET_DIR/debug/integration
$CARGO_TARGET_DIR/debug/integration --show-output
env:
INTEGRATION: ${{ matrix.integration }}

4 changes: 4 additions & 0 deletions tests/integration.rs
View file
Original file line number Diff line number Diff line change
@@ -65,6 +65,10 @@ fn integration_test() {
.expect("unable to run clippy");

let stderr = String::from_utf8_lossy(&output.stderr);

// debug:
eprintln!("{stderr}");
Comment on lines +69 to +70
Copy link
Member

Choose a reason for hiding this comment

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

Is this actually a leftover for debugging, or do you want to have this in the tests? From the added --show-output flag, I assume this is left here intentionally.

I think this could be quite useful, just want to make sure it wasn't forgotten about.

Copy link
Member Author

Choose a reason for hiding this comment

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

No, this makes us print the clippy command stderr, but it would still be hidden by the testing harness by default, which is why we also need the other flag.

I think its nice to be verbose by default here so we can see what the problem is right away,in case there are any.

Copy link
Member

Choose a reason for hiding this comment

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

I thought so. Makes sense, let's ship it :shipit:


if let Some(backtrace_start) = stderr.find("error: internal compiler error") {
static BACKTRACE_END_MSG: &str = "end of query stack";
let backtrace_end = stderr[backtrace_start..]