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: workflows for local runs and GH actions #18

Closed
wants to merge 10 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
60 changes: 60 additions & 0 deletions .github/workflows/ci.yml
Copy link
Member

Choose a reason for hiding this comment

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

It might be useful to define separate jobs for running tests, linting and formatting checks. That way they can run in parallel, and it is easier to see at a glance where a problem arose. See here for an example.

Of course, for each job you only need to install what is really required for each job (e.g., JDK and Funnel will likely not be needed for linting and formatting).

Btw, this workflow also highlights the issues that may quickly arise when writing your unit/integration tests in a way that they depend on other services. Not only running the tests, but also setting them up (e.g. installing Rust, Go, JDK - and the list will quickly get longer the more services you require for testing) will take increasingly more time.

I think it is important to consider and implement a scalable testing strategy early, e.g.:

  • Use multiple GitHub Actions workflows for static code analysis (e.g., linting, formatting, code quality), different test types (see below) and other checks (PR validation, security etc.); again, see here for an example
  • Keep unit and basic integration tests dependency free (by monkey patching and mocking expected responses), so as to keep the turnaround time for tests low during regular development.
  • Define potentially long-running end-to-end tests to comprehensively test client functionalities against real world reference implementations for each client (e.g., Funnel for TES); only run those tests when staging releases - in a separate GitHub Actions workflow.

Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
name: GitHub CI/CD Workflow

on: [push, pull_request]

jobs:
build:

runs-on: ubuntu-latest
container:
image: ubuntu:24.04
env:
OPENSSL_DIR: /usr/include/openssl
OPENSSL_LIB_DIR: /usr/lib/x86_64-linux-gnu
OPENSSL_INCLUDE_DIR: /usr/include/openssl

steps:
- name: Install dependencies
run: |
apt-get update
apt-get install -y curl git build-essential libssl-dev

- uses: actions/checkout@v4 # checkout the repository
- name: Install Rust
run: |
curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y
echo "$HOME/.cargo/bin" >> $GITHUB_PATH

- name: Set up JDK 11 # required for build-models.sh
uses: actions/setup-java@v3
with:
distribution: 'adopt'
java-version: '11'

- name: Install Go
uses: actions/setup-go@v4
with:
go-version: '^1.22'
- name: Install Funnel
run: |
if [ -d "funnel" ]; then rm -Rf funnel; fi
git clone https://github.com/ohsu-comp-bio/funnel.git
cd funnel && make && make install && cd ..
- uses: actions/checkout@v4 # checkout the repository
- name: Build models
run: |
bash ./build-models.sh
- name: Build
run: |
cargo build --verbose
- name: Run tests
run: |
bash ./run-tests.sh
- name: Lint
run: |
cargo clippy -- -D warnings
- name: Format
run: |
cargo fmt -- ./lib/src/serviceinfo/models/*.rs # workaround to fix autogenerated code formatting
cargo fmt -- ./lib/src/tes/models/*.rs
cargo fmt -- --check
98 changes: 98 additions & 0 deletions .github/workflows/local.yml
Copy link
Member

Choose a reason for hiding this comment

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

Minus the caching (which would also be useful when not triggered locally, no?), this is essentially the same workflow as the previous one. Couldn't you just add workflow_dispatch as a trigger to the first workflow?

It is a considerable maintenance burden to keep multiple copies of (virtually) the same code in sync - sooner or later they will diverge!

Also, please document (e.g., in README.md) how to trigger workflow execution locally.

Copy link
Member

Choose a reason for hiding this comment

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

I've just seen that triggering the workflow locally is documented in #29, so I guess you can ignore that.

Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
name: Local CI/CD Workflow

on: workflow_dispatch

jobs:
build:

runs-on: ubuntu-latest
container:
image: ubuntu:24.04
env:
OPENSSL_DIR: /usr/include/openssl
OPENSSL_LIB_DIR: /usr/lib/x86_64-linux-gnu
OPENSSL_INCLUDE_DIR: /usr/include/openssl

steps:

- name: Cache Rust dependencies
uses: actions/cache@v3
with:
path: ~/.cargo
key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }}
restore-keys: ${{ runner.os }}-cargo-

- name: Cache Rust build output
uses: actions/cache@v3
with:
path: target
key: ${{ runner.os }}-build-${{ hashFiles('**/Cargo.lock') }}
restore-keys: ${{ runner.os }}-build-

- name: Cache Maven dependencies
uses: actions/cache@v3
with:
path: ~/.m2/repository
key: ${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }}
restore-keys: ${{ runner.os }}-maven-

- name: Cache Go modules
uses: actions/cache@v3
with:
path: ~/.cache/go-build
key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}
restore-keys: ${{ runner.os }}-go-

- name: Cache Funnel dependencies
uses: actions/cache@v3
with:
path: ~/funnel/build
key: ${{ runner.os }}-funnel-${{ hashFiles('**/funnel/*') }}
restore-keys: ${{ runner.os }}-funnel-

- uses: actions/checkout@v4 # checkout the repository
- name: Install Rust
run: |
curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y
echo "$HOME/.cargo/bin" >> $GITHUB_PATH

- name: Set up JDK 11 # required for build-models.sh
uses: actions/setup-java@v3
with:
distribution: 'adopt'
java-version: '11'

- name: Install Go
uses: actions/setup-go@v4
with:
go-version: '^1.22'
- name: Install Funnel
run: |
if [ -d "funnel" ]; then rm -Rf funnel; fi
git clone https://github.com/ohsu-comp-bio/funnel.git
cd funnel && make && make install && cd ..
- uses: actions/checkout@v4 # checkout the repository
- name: Build models
run: |
. $HOME/.cargo/env
bash ./build-models.sh
- name: Build
run: |
. $HOME/.cargo/env
cargo build --verbose
- name: Run tests
run: |
. $HOME/.cargo/env
bash ./run-tests.sh
- name: Lint
run: |
. $HOME/.cargo/env
cargo clippy -- -D warnings
- name: Format
run: |
. $HOME/.cargo/env
# rustup install nightly – fails for some reason
aaravm marked this conversation as resolved.
Show resolved Hide resolved
# rustup default nightly
cargo fmt -- ./lib/src/serviceinfo/models/*.rs # workaround to fix autogenerated code formatting
cargo fmt -- ./lib/src/tes/models/*.rs
cargo fmt -- --check # --config-path ./rustfmt.toml
Loading