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 CD workflows #4

Open
wants to merge 9 commits into
base: release_candidate_1
Choose a base branch
from

Conversation

laruh
Copy link
Member

@laruh laruh commented Oct 6, 2024

  • provides lint, fmt checks and unit tests in workflows
  • fixes fmt and clippy

@laruh laruh mentioned this pull request Oct 8, 2024
7 tasks
@laruh laruh requested a review from Alrighttt October 25, 2024 13:44
…e_candidate_1-ci-cd

# Conflicts:
#	src/tests/spend_policy.rs
@laruh
Copy link
Member Author

laruh commented Oct 27, 2024

@Alrighttt Hi, one of the tests is failing. Pinging you in case you didn’t see it
https://github.com/laruh/sia-rust/actions/runs/11537715300/job/32115426385#step:5:414

---- transport::client::native::tests::test_api_events stdout ----
thread 'transport::client::native::tests::test_api_events' panicked at 'called `Result::unwrap()` on an `Err` value: UnexpectedHttpStatus { status: 404, body: "event not found\n" }', src/transport/client/native.rs:140:46
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

@Alrighttt
Copy link
Member

This is utilizing an unmaintained project, actions-rs. Could you please research an alternative?

The github runner should have rustup by default. It doesn't seem necessary to use an action.

    - name: Install Rust toolchain
      run: |
        rustup install nightly-2023-06-01 --profile minimal --component clippy rustfmt
        rustup default nightly-2023-06-01

I believe it could be as simple as this.

…e_candidate_1-ci-cd

# Conflicts:
#	src/transport/client/helpers.rs
#	src/types/atomic_swap.rs
@laruh
Copy link
Member Author

laruh commented Oct 28, 2024

This is utilizing an unmaintained project, actions-rs. Could you please research an alternative?

The github runner should have rustup by default. It doesn't seem necessary to use an action.

    - name: Install Rust toolchain
      run: |
        rustup install nightly-2023-06-01 --profile minimal --component clippy rustfmt
        rustup default nightly-2023-06-01

I believe it could be as simple as this.

What you've provided is a manual toolchain installation, it has the same result as steps made in actions.

Did you re check error message, it has nothing to do with actions-rs/toolchain@v1 (it is for more convenient toolchain installation setup in yml file and nothing more)

test_api_events' panicked at 'called `Result::unwrap()` on an `Err` value: UnexpectedHttpStatus { status: 404, body: "event not found\n" }', src/transport/client/native.rs:140:46

run cargo test command locally
image

@Alrighttt
Copy link
Member

This is utilizing an unmaintained project, actions-rs. Could you please research an alternative?
The github runner should have rustup by default. It doesn't seem necessary to use an action.

    - name: Install Rust toolchain
      run: |
        rustup install nightly-2023-06-01 --profile minimal --component clippy rustfmt
        rustup default nightly-2023-06-01

I believe it could be as simple as this.

What you've provided is a manual toolchain installation, it has the same result as steps made in actions.

Did you re check error message, it has nothing to do with actions-rs/toolchain@v1 (it is for more convenient toolchain installation setup in yml file and nothing more)

test_api_events' panicked at 'called `Result::unwrap()` on an `Err` value: UnexpectedHttpStatus { status: 404, body: "event not found\n" }', src/transport/client/native.rs:140:46

run cargo test command locally image

actions-rs/cargo#59 (comment)
The actions-rs project is no longer maintained. Find an alternative please.

The error can be tagged with #[ignore] or deleted. It's related to the task item in #3 : "Migrate docker test framework from Komodo DeFi Framework into Sia Rust. There are tests throughout the codebase that rely on a hardcoded URL to a walletd index node. Remove these in favor of mocked API or walletd docker containers."

@laruh
Copy link
Member Author

laruh commented Oct 28, 2024

@Alrighttt sorry I misread your first message, I thought it was the answer to my note about failing test
Fixed toolchain installation here fbfe97a

as for test: delete it or ignore, its up to you in release candidate branch

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

Successfully merging this pull request may close these issues.

2 participants