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

Move tests into the tests directory & run tests on GH Actions #123

Merged
merged 3 commits into from
Aug 8, 2023

Conversation

sampoder
Copy link
Member

The successor to #121... rebasing is doing my head in but full steam ahead! This PR works towards resolving #82 & #60.

@sampoder sampoder marked this pull request as draft June 29, 2023 05:07
@sampoder sampoder force-pushed the move-tests branch 2 times, most recently from c5e0467 to 470490d Compare June 29, 2023 18:21
@conradev conradev marked this pull request as ready for review June 29, 2023 18:22
Copy link
Collaborator

@conradev conradev left a comment

Choose a reason for hiding this comment

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

We need to fix two things here:

  • The test command should match the build command more. We should test all targets and features. We should also make explicit which targets we are testing in the matrix configuratio (i.e. the host target)

  • The test command seemingly recompiles the crate, I'd be curious why it can't re-use the previous compile (might be for the above reason)

@sampoder
Copy link
Member Author

sampoder commented Jul 3, 2023

Let me look into recompilation, that'll speed things up a lot when removed!

@@ -60,3 +60,6 @@ jobs:
- name: Build
shell: bash
run: cargo build --verbose --workspace --all-features --target ${{ join(matrix.targets, ' --target ') }}
- name: Post-Build Tests
shell: bash
run: cargo test
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
run: cargo test
run: cargo test --verbose --workspace --all-features --target ${{ join(matrix.targets, ' --target ') }}

@conradev is this what you're looking for?

@sampoder
Copy link
Member Author

sampoder commented Jul 3, 2023

The test command seemingly recompiles the crate, I'd be curious why it can't re-use the previous compile (might be for the above reason)

Based on sagiegurari/cargo-make#251, it seems like removing cargo build [...] and just doing cargo test [...] would achieve what we'd want.

Also run these tests on Github Actions as part of the PR request
flow.
@conradev conradev force-pushed the move-tests branch 3 times, most recently from 290284e to d6d490c Compare August 5, 2023 16:33
@conradev conradev enabled auto-merge (rebase) August 5, 2023 16:33
@conradev conradev merged commit d821f3d into main Aug 8, 2023
9 checks passed
@conradev conradev deleted the move-tests branch August 8, 2023 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants