-
Notifications
You must be signed in to change notification settings - Fork 3
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
[TRUNK-13813] Smoke test PRs #240
Conversation
Adds the smoke test to prs. Does not notify slack to avoid noise.
😎 Merged successfully - details. |
TRUNK_PUBLIC_API_ADDRESS: https://api.trunk-staging.io | ||
shell: bash | ||
run: | | ||
./target/x86_64-unknown-linux-musl/release/trunk-analytics-cli test \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to run on latest build or the currently latest in the github action. I could see value for both but I think we definitely at least want the latter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is triggered on prs, I want it running on the pr branch. Unless I've horribly misunderstood workflows, using the checkout with ref: ${{ github.base_ref }}
should be putting us there, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this needs to be a separate test then, but I would really like to test staging services against what is latest in the GH action
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, agreed. We want testing of main
and we want testing of a "pinned" version for both staging and prod.
Additionally, I don't mind a PR smoke test, but I don't think it's as needed right now as the above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, gotcha. I'll pivot this to do that instead and post to channel.
- name: Setup Rust & Cargo | ||
uses: ./.github/actions/setup_rust_cargo | ||
|
||
- name: Install Nextest | ||
shell: bash | ||
run: | | ||
cargo install --version 0.9.85 cargo-nextest --locked | ||
|
||
- name: Install cmake | ||
shell: bash | ||
run: | | ||
sudo apt-get install cmake -y | ||
|
||
- name: Run succeeding test | ||
env: | ||
TRUNK_PUBLIC_API_ADDRESS: https://api.trunk-staging.io | ||
shell: bash | ||
run: | | ||
./target/x86_64-unknown-linux-musl/release/trunk-analytics-cli test \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If most of these steps are shared across smoke test workflows, let's make an action to keep things DRY
analytics-cli/.github/workflows/smoke_test.yml
Lines 24 to 46 in 9fbb8d4
- name: Setup Rust & Cargo | |
uses: ./.github/actions/setup_rust_cargo | |
- name: Install Nextest | |
shell: bash | |
run: | | |
cargo install --version 0.9.85 cargo-nextest | |
- name: Install cmake | |
shell: bash | |
run: | | |
sudo apt-get install cmake -y | |
- name: Run succeeding test | |
env: | |
TRUNK_PUBLIC_API_ADDRESS: https://api.trunk-staging.io | |
shell: bash | |
run: | | |
./trunk-analytics-cli test \ | |
--org-url-slug trunk-staging-org \ | |
--junit-paths ${{ github.workspace }}/target/**/*junit.xml \ | |
--token ${{ secrets.TRUNK_STAGING_ORG_API_TOKEN }} \ | |
cargo nextest run -p smoke-test --profile ci |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point; refactored
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
Adds the smoke test to prs. Does not notify slack to avoid noise.