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

CI workflow doesn't test toolchain targets correctly #20

Closed
Mrmaxmeier opened this issue Aug 5, 2021 · 3 comments
Closed

CI workflow doesn't test toolchain targets correctly #20

Mrmaxmeier opened this issue Aug 5, 2021 · 3 comments

Comments

@Mrmaxmeier
Copy link

Mrmaxmeier commented Aug 5, 2021

This build step doesn't work as expected because rustup does not manage rustc's default target:

- name: Install toolchain
  uses: actions-rs/toolchain@v1
  with:
    toolchain: stable
    profile: minimal
    target: ${{ matrix.target }}
    override: true

- name: Run tests
  run: cargo test

The tests fail when the target is set explicitly:

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 380f1d0..b3e4c16 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -49,7 +49,7 @@ jobs:
         override: true
 
     - name: Run tests
-      run: cargo test
+      run: cargo test --target ${{ matrix.target }}
 
   test-macos-catalina:
     runs-on: macos-10.15
@@ -82,7 +82,7 @@ jobs:
         override: true
 
     - name: Run tests
-      run: cargo test
+      run: cargo test --target ${{ matrix.target }}

i686-unknown-linux-musl seems to run into a 32-bit truncation issue:

---- test::map_offset stdout ----
thread 'test::map_offset' panicked at 'attempt to subtract with overflow', src/lib.rs:179:23
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

and i686-unknown-linux-gnu fails during linking probably because 32-bit glibc isn't installed or something.

Here's a sample run (not sure if it'll survive force-pushes): https://github.com/Mrmaxmeier/memmap2-rs/runs/3251483567 (xref #21)

@RazrFalcon
Copy link
Owner

Isn't override: true changes the default target?

get_len indeed should be fixed. Are you planning a pull request?

@Mrmaxmeier
Copy link
Author

Isn't override: true changes the default target?

override: true will only set the toolchain (stable/nightly) but not the target. There's probably a bunch of other projects that miss this :/

get_len indeed should be fixed. Are you planning a pull request?

Wasn't sure if the implementation or the tests need fixing.. I'll leave this one to you 🙂

@RazrFalcon
Copy link
Owner

override: true will only set the toolchain (stable/nightly) but not the target.

Yes, makes sense. I'm rarely using non-default target.

Ok, I will fix it then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants