Skip to content

Commit 72c47b4

Browse files
authored
Migrate CI workflow to scripts, allow ci/validate (#1377)
### What does this PR do? This commit builds on the insights gained from the automated bollard dependency update and moves our CI correctness testing into locally executable scripts with a single 'validate' to call the whole suite. CLAUDE.md is updated to prefer the use of `ci/validate`.
1 parent 5084142 commit 72c47b4

File tree

13 files changed

+371
-49
lines changed

13 files changed

+371
-49
lines changed

.github/workflows/ci.yml

Lines changed: 23 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,13 @@ on:
77
- main
88

99
jobs:
10+
shellcheck:
11+
name: Shellcheck
12+
runs-on: ubuntu-latest
13+
steps:
14+
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
15+
- run: ci/shellcheck
16+
1017
rust_actions:
1118
name: Rust Actions (Check/Fmt/Clippy)
1219
runs-on: ${{ matrix.os }}
@@ -20,17 +27,17 @@ jobs:
2027
protobuf: true
2128
fuse: true
2229
components: ""
23-
command: cargo check --locked --all-features
30+
command: ci/check
2431
- tool: fmt
2532
protobuf: true
2633
fuse: true
2734
components: "rustfmt"
28-
command: cargo fmt --all -- --check
35+
command: ci/fmt
2936
- tool: clippy
3037
protobuf: true
3138
fuse: true
3239
components: "clippy"
33-
command: cargo clippy --all-features
40+
command: ci/clippy
3441
steps:
3542
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
3643
- uses: actions-rust-lang/setup-rust-toolchain@9d7e65c320fdb52dcd45ffaa68deb6c02c8754d9 # v1.10
@@ -49,7 +56,12 @@ jobs:
4956
runs-on: ubuntu-latest
5057
steps:
5158
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
52-
- uses: EmbarkStudios/cargo-deny-action@34899fc7ba81ca6268d5947a7a16b4649013fea1 # v2.0.11
59+
- uses: actions-rust-lang/setup-rust-toolchain@9d7e65c320fdb52dcd45ffaa68deb6c02c8754d9 # v1.10
60+
- name: Install Protobuf
61+
uses: ./.github/actions/install-protobuf
62+
- name: Install FUSE
63+
uses: ./.github/actions/install-fuse
64+
- run: ci/deny
5365

5466
test:
5567
name: Test Suite
@@ -63,7 +75,7 @@ jobs:
6375
uses: ./.github/actions/install-fuse
6476
- name: Install nextest
6577
uses: taiki-e/install-action@49be99c627fae102cb8c86414e9605869641782a # nextest
66-
- run: cargo nextest run --all-features
78+
- run: ci/test
6779

6880
integration-test:
6981
name: Integration Tests
@@ -75,7 +87,7 @@ jobs:
7587
uses: ./.github/actions/install-protobuf
7688
- name: Install FUSE
7789
uses: ./.github/actions/install-fuse
78-
- run: cargo test --locked -p sheepdog
90+
- run: ci/integration-test
7991
timeout-minutes: 30
8092

8193
kani:
@@ -93,43 +105,23 @@ jobs:
93105
uses: ./.github/actions/install-fuse
94106
- name: Install kani
95107
run: cargo install kani-verifier
96-
- run: cargo kani --solver cadical
97-
working-directory: ${{ matrix.crate }}
108+
- run: ci/kani ${{ matrix.crate }}
98109
timeout-minutes: 30
99110

100-
loom:
101-
name: Loom Proofs
111+
buf:
102112
runs-on: ubuntu-latest
103-
strategy:
104-
matrix:
105-
crate: [lading_signal]
106113
steps:
114+
# Check our protobufs for lint cleanliness and for lack of breaking
115+
# changes
107116
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
108117
- uses: actions-rust-lang/setup-rust-toolchain@9d7e65c320fdb52dcd45ffaa68deb6c02c8754d9 # v1.10
109118
- name: Install Protobuf
110119
uses: ./.github/actions/install-protobuf
111120
- name: Install FUSE
112121
uses: ./.github/actions/install-fuse
113-
- name: Install nextest
114-
uses: taiki-e/install-action@49be99c627fae102cb8c86414e9605869641782a # nextest
115-
- run: RUSTFLAGS="--cfg loom" cargo nextest run --release
116-
working-directory: ${{ matrix.crate }}
117-
timeout-minutes: 30
118-
119-
buf:
120-
runs-on: ubuntu-latest
121-
steps:
122-
# Check our protobufs for lint cleanliness and for lack of breaking
123-
# changes
124-
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
125122
- name: buf-setup
126123
uses: bufbuild/buf-setup-action@a47c93e0b1648d5651a065437926377d060baa99 # v1.50.0
127-
- name: buf-lint
128-
uses: bufbuild/buf-lint-action@06f9dd823d873146471cfaaf108a993fe00e5325 # v1.1.1
129-
- name: buf-breaking
130-
uses: bufbuild/buf-breaking-action@c57b3d842a5c3f3b454756ef65305a50a587c5ba # v1.1.4
131-
with:
132-
against: 'https://github.com/datadog/lading.git#branch=main'
124+
- run: ci/buf
133125

134126
actionlint:
135127
runs-on: ubuntu-latest

CLAUDE.md

Lines changed: 125 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,17 @@
1+
# Architecture Overview
2+
3+
Lading measures performance of long-running programs (daemons) using synthetic,
4+
repeatable load generation. It operates with three components:
5+
6+
- **Generators**: Create deterministic load and push to targets
7+
- **Targets**: Programs being tested (run as subprocesses by lading)
8+
- **Blackholes**: Optionally receive output from targets
9+
10+
Data flows: Generator → Target → Blackhole
11+
12+
Key principle: Pre-compute everything possible. This minimizes runtime overhead
13+
and interference with the target being measured.
14+
115
# Design Goals
216

317
Lading is a performance tool. It's behavior has consequences on the development
@@ -15,22 +29,45 @@ this means that lading will pre-allocate where possible, will explicitly thread
1529
randomness and other sources of non-determinism into code paths and will
1630
preference algorithms that have better worst-case behavior.
1731

18-
# Code Style
32+
## Performance Guidance
33+
34+
Always consider performance implications of changes. Focus on "obviously fast" patterns:
35+
- Pre-allocate collections when size is known
36+
- Avoid allocations in hot paths
37+
- Choose algorithms with good worst-case behavior over average-case
38+
- Prefer bounded operations over unbounded ones
39+
- Avoid unnecessary cloning or copying
40+
41+
Do NOT dive into performance optimization rabbit holes without profiling data.
42+
Benchmarks alone are insufficient - real performance work requires profiling.
43+
Stick to obviously correct performance patterns.
1944

20-
This project is subject attempts to automate code style to a great degree, using
21-
these tools:
45+
## Error Handling
2246

23-
* `cargo clippy`
24-
* `cargo fmt`
47+
Lading MUST NOT panic. Use controlled shutdowns only. All errors should be
48+
propagated using Result types. Panics are only acceptable for truly exceptional
49+
circumstances that cannot be handled (e.g., fundamental invariant violations that
50+
indicate memory corruption).
51+
52+
When handling errors:
53+
- Always use Result<T, E> for fallible operations
54+
- Propagate errors up to where they can be meaningfully handled
55+
- Provide context when wrapping errors
56+
- Design for graceful degradation where possible
57+
58+
# Code Style
59+
60+
This project enforces code style through automated tooling. Use `ci/validate` to
61+
check style compliance - it will run formatting and linting checks for you.
2562

2663
We do not allow for warnings: all warnings are errors. Deprecation warnings MUST
2764
be treated as errors. Lading is written in a "naive" style where abstraction is
2865
not preferred if a duplicated pattern will satisfy. Our reasoning for this is it
2966
makes ramp-up for new engineers easier: all you must do is follow the pattern,
3067
not internalize a complex type hierarchy. There are obvious places in the code
3168
base where replicated patterns have been made into abstractions -- we follow the
32-
"shape" rule, if you have three or more repeats, make a jig -- but we do not
33-
start there.
69+
"shape" rule: if you have three or more repeats, make a jig. This is a hard rule:
70+
3+ repetitions = create an abstraction. Less than 3 = duplicate the pattern.
3471

3572
Lading is written to be as easy to contribute to as possible. We ask that any
3673
dependency used in the project in more than one crate be present in the
@@ -46,28 +83,98 @@ or two of code. Do add comments that explain the "why" of a block of code, how
4683
it functions or is unusual in a way that an experienced engineer might not
4784
understand.
4885

86+
Always add comments for:
87+
- Non-obvious performance optimizations
88+
- Complex state machines or control flow
89+
- Unusual algorithm choices
90+
- Workarounds for external limitations
91+
- Any code that would make an experienced engineer pause and wonder "why?"
92+
4993
Crate versions are always given as XX.YY and not XX.YY.ZZ.
5094

5195
# Testing
5296

53-
Where possible lading prefers property tests over unit tests and in especially
54-
critical components we require proofs. We use
55-
[proptest](https://github.com/proptest-rs/proptest) in this project for property
56-
tests and the [kani](https://github.com/model-checking/kani) proof tool.
97+
ALWAYS prefer property tests over unit tests. Unit tests are insufficient for
98+
lading's requirements. We use [proptest](https://github.com/proptest-rs/proptest)
99+
for property testing.
100+
101+
Critical components are those which must function correctly or lading itself
102+
cannot function. These require proofs using [kani](https://github.com/model-checking/kani):
103+
- Throttling (MUST be correct - lading is used to make throughput claims)
104+
- Core data generation
105+
- Fundamental algorithms
57106

58-
## Workflow
107+
The throttle is especially critical: incorrect throttling invalidates all
108+
performance claims made using lading.
59109

60-
Changes to lading are subject this flow:
110+
When writing tests:
111+
- Default to property tests
112+
- Unit tests are only acceptable for simple pure functions
113+
- Think about the properties your code should maintain
114+
- Test invariants, not specific examples
61115

62-
* `cargo check`
63-
* `cargo clippy`
64-
* `cargo nextest run`
116+
# CRITICAL: Validation Workflow
65117

66-
Proofs must be run with the `cargo kani` tool in the crate where proofs reside.
118+
You MUST run `ci/validate` after making any code changes. This is not optional.
119+
The script runs all checks in optimal order and exits on first failure.
67120

121+
Do not run individual cargo commands - use the ci scripts instead:
122+
- Use `ci/validate` for full validation
123+
- Use `ci/fmt` instead of `cargo fmt`
124+
- Use `ci/check` instead of `cargo check`
125+
- Use `ci/clippy` instead of `cargo clippy`
126+
- Use `ci/test` instead of `cargo nextest run`
127+
- Use `ci/kani <crate>` for kani proofs (valid crates: lading_throttle, lading_payload)
128+
- Use `ci/outdated` instead of `cargo outdated`
68129

69130
# Tools
70131

71-
To identify outdated dependencies: `cargo outdated --root-deps-only`.
132+
To identify outdated dependencies: Use `ci/outdated`
72133

73134
To run micro-benchmarks: `cargo criterion`
135+
136+
## Dependencies
137+
138+
Due to lading's unusual constraints (performance, determinism, correctness),
139+
we often must implement functionality rather than use external crates.
140+
141+
Before suggesting a new dependency, consider:
142+
- Does it meet our performance requirements?
143+
- Is it deterministic?
144+
- Does it give us sufficient control?
145+
- Is the functionality core to lading's purpose?
146+
147+
When in doubt, implement rather than import.
148+
149+
## Common Patterns and Anti-Patterns
150+
151+
### DO:
152+
- Pre-allocate buffers and collections when size is known
153+
- Use bounded channels/queues over unbounded
154+
- Design for worst-case scenarios
155+
- Use fixed-size data structures where possible
156+
- Propagate errors up to where they can be handled meaningfully
157+
158+
### DON'T:
159+
- Use `.unwrap()` or `.expect()` (these panic)
160+
- Allocate in hot paths
161+
- Use unbounded growth patterns
162+
- Add external dependencies without careful consideration
163+
- Implement complex abstractions for fewer than 3 use cases
164+
165+
# Key Reminders for Claude
166+
167+
1. ALWAYS use `ci/validate` after code changes - never skip this
168+
2. Do NOT run cargo commands directly - use the ci/ scripts
169+
3. When users ask about build/test failures, suggest running `ci/validate`
170+
4. If users are confused about project correctness criteria, point them to `ci/validate`
171+
5. The ci/ scripts are the single source of truth for validation
172+
6. Always consider performance - suggest "obviously fast" patterns
173+
7. Do NOT suggest performance optimizations without profiling data
174+
8. When in doubt, prefer pre-allocation and good worst-case behavior
175+
9. NEVER suggest code that could panic - use Result types
176+
10. Be skeptical of external dependencies - we often need custom implementations
177+
11. Remember the throttle is critical - any changes there need extra scrutiny
178+
12. Generators must be deterministic - no randomness without explicit seeding
179+
13. Pre-compute in initialization, not in hot paths
180+
14. Think about how your code affects the measurement of the target

ci/buf

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
#!/usr/bin/env bash
2+
set -o errexit
3+
set -o nounset
4+
set -o pipefail
5+
6+
# Check if buf is installed
7+
if ! command -v buf &> /dev/null; then
8+
echo "buf is not installed. Please install it first."
9+
echo "See: https://docs.buf.build/installation"
10+
exit 1
11+
fi
12+
13+
echo "Running buf lint..."
14+
buf lint
15+
16+
echo "Running buf breaking check..."
17+
buf breaking --against 'https://github.com/datadog/lading.git#branch=main'
18+
19+
echo "Buf checks passed!"

ci/check

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
#!/usr/bin/env bash
2+
set -o errexit
3+
set -o nounset
4+
set -o pipefail
5+
6+
echo "Running cargo check..."
7+
cargo check --locked --all-features
8+
9+
echo "Cargo check passed!"

ci/clippy

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
#!/usr/bin/env bash
2+
set -o errexit
3+
set -o nounset
4+
set -o pipefail
5+
6+
echo "Running cargo clippy..."
7+
cargo clippy --all-features
8+
9+
echo "Cargo clippy passed!"

ci/deny

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
#!/usr/bin/env bash
2+
set -o errexit
3+
set -o nounset
4+
set -o pipefail
5+
6+
# Check if cargo-deny is installed
7+
if ! command -v cargo-deny &> /dev/null; then
8+
echo "cargo-deny is not installed. Installing it now..."
9+
cargo install cargo-deny
10+
fi
11+
12+
echo "Running cargo deny..."
13+
cargo deny check
14+
15+
echo "Cargo deny passed!"

ci/fmt

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
#!/usr/bin/env bash
2+
set -o errexit
3+
set -o nounset
4+
set -o pipefail
5+
6+
echo "Running cargo fmt..."
7+
cargo fmt --all -- --check
8+
9+
echo "Cargo fmt passed!"

ci/integration-test

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
#!/usr/bin/env bash
2+
set -o errexit
3+
set -o nounset
4+
set -o pipefail
5+
6+
echo "Running integration tests..."
7+
cargo test --locked -p sheepdog
8+
9+
echo "Integration tests passed!"

0 commit comments

Comments
 (0)