Skip to content

test: cover lifecycle init/post_upgrade argument validation#159

Merged
mbjorkqvist merged 4 commits into
mainfrom
test/lifecycle-arg-validation
Jun 23, 2026
Merged

test: cover lifecycle init/post_upgrade argument validation#159
mbjorkqvist merged 4 commits into
mainfrom
test/lifecycle-arg-validation

Conversation

@mbjorkqvist

Copy link
Copy Markdown
Contributor

Closes a test gap surfaced during the repo review: lifecycle.rs had no mod tests, leaving the operator-facing init/upgrade arg-validation and snapshot-recovery branches — the code paths exercised at the worst possible time, during an upgrade — entirely untested.

Adds a tests module covering:

  • init traps when handed an Upgrade argument.
  • post_upgrade traps when the state snapshot is missing (i.e. pre_upgrade trapped or was skipped).
  • post_upgrade traps when handed an Init argument.

Test-only; no production code changed.

Adds a tests module to lifecycle.rs exercising the previously-untested
argument-validation and recovery branches:
- init traps when given an Upgrade argument.
- post_upgrade traps when the state snapshot is missing (pre_upgrade
  trapped or was skipped).
- post_upgrade traps when given an Init argument.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 18, 2026 09:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds a #[cfg(test)] unit test module to canister/src/lifecycle.rs to cover upgrade lifecycle argument validation and snapshot-missing failure behavior, addressing previously untested operator-critical paths during upgrades.

Changes:

  • Add a test ensuring init panics when passed an Upgrade argument.
  • Add a test ensuring post_upgrade panics when no state snapshot is available.
  • Add a test ensuring post_upgrade panics when passed an Init argument (with a snapshot present so validation is reached).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions

github-actions Bot commented Jun 18, 2026

Copy link
Copy Markdown

canbench 🏋 (dir: canister) 6894f40 2026-06-23 06:15:43 UTC

canister/canbench_results.yml is up to date
📦 canbench_results_benchmark.csv available in artifacts

---------------------------------------------------

Summary:
  instructions:
    status:   No significant changes 👍
    counts:   [total 12 | regressed 0 | improved 0 | new 0 | unchanged 12]
    change:   [max +794.90K | p75 +63.93K | median +154 | p25 -220.35K | min -1.82M]
    change %: [max +0.47% | p75 +0.15% | median +0.03% | p25 -0.11% | min -1.02%]

  heap_increase:
    status:   No significant changes 👍
    counts:   [total 12 | regressed 0 | improved 0 | new 0 | unchanged 12]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

  stable_memory_increase:
    status:   No significant changes 👍
    counts:   [total 12 | regressed 0 | improved 0 | new 0 | unchanged 12]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

---------------------------------------------------

Only significant changes:
| status | name                               | calls |    ins |  ins Δ% | HI |  HI Δ% | SMI |  SMI Δ% |
|--------|------------------------------------|-------|--------|---------|----|--------|-----|---------|
|   -    | bench_write_events::AddTradingPair |     1 | 16.95K |  -2.04% |  0 |  0.00% |   0 |   0.00% |
|   -    | bench_write_events::Deposit        |     1 | 10.30K |  -2.15% |  0 |  0.00% |   0 |   0.00% |
|   -    | bench_write_events::Init           |     1 | 23.13K |  -2.71% |  0 |  0.00% |   0 |   0.00% |

ins = instructions, HI = heap_increase, SMI = stable_memory_increase, Δ% = percent change

---------------------------------------------------
CSV results saved to canbench_results.csv

mbjorkqvist and others added 2 commits June 18, 2026 09:47
Follow the repo convention: every module keeps its tests in a separate
tests.rs wired via `mod tests;`, not an inline test module. Convert
lifecycle.rs into lifecycle/mod.rs and move the argument-validation tests
into lifecycle/tests.rs.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 22, 2026 08:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@mbjorkqvist mbjorkqvist marked this pull request as ready for review June 22, 2026 09:00
@mbjorkqvist mbjorkqvist requested a review from a team as a code owner June 22, 2026 09:01
@zeropath-ai

zeropath-ai Bot commented Jun 22, 2026

Copy link
Copy Markdown

No security or compliance issues detected. Reviewed everything up to c756abd.

Security Overview
Detected Code Changes
Change Type Relevant files
Refactor ► canister/src/lifecycle.rs
    Rename lifecycle.rs to mod.rs
    Add #[cfg(test)] for tests module
Enhancement ► canister/src/lifecycle/tests.rs
    Add tests for init function
    Add tests for post_upgrade function

@gregorydemay gregorydemay left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @mbjorkqvist !

Comment thread canister/src/lifecycle/mod.rs Outdated
Place `#[cfg(test)] mod tests;` right after the imports rather than at
the end of the file, matching the convention used elsewhere (e.g.
`order/queue/mod.rs`). Addresses review nit on #159.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@mbjorkqvist mbjorkqvist added this pull request to the merge queue Jun 23, 2026
Merged via the queue into main with commit ee41173 Jun 23, 2026
15 checks passed
@mbjorkqvist mbjorkqvist deleted the test/lifecycle-arg-validation branch June 23, 2026 06:28
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.

3 participants