Add CI test infrastructure and code coverage tools#34
Conversation
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| uses: taiki-e/install-action@cargo-llvm-cov | ||
|
|
||
| - name: Run tests with coverage | ||
| run: cargo llvm-cov --lcov --output-path lcov.info |
There was a problem hiding this comment.
Suggestion: add restore-keys so a cache miss on a lock file change still gets a partial hit, avoiding a full cold rebuild every time dependencies change.
| run: cargo llvm-cov --lcov --output-path lcov.info | |
| key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} | |
| restore-keys: | | |
| ${{ runner.os }}-cargo- |
| .unwrap() | ||
| .execute(Print(format!("{login_url}\n"))) | ||
| .unwrap() | ||
| .execute(ResetColor) |
There was a problem hiding this comment.
The exchange_token function has a hidden side effect: it saves the API key to disk via config::save_api_key. The caller in login() destructures LoginResult::Success { workspace, .. } and ignores the returned token field — because it was already persisted inside here. This makes the token field in LoginResult::Success redundant and the function contract surprising.
Non-blocking, but consider either naming this exchange_and_save_token or removing the token field from LoginResult::Success since callers don't need it.
| let tmp = tempfile::tempdir().unwrap(); | ||
| // SAFETY: tests are serialized via ENV_LOCK mutex, so no concurrent env mutation. | ||
| unsafe { std::env::set_var("HOTDATA_CONFIG_DIR", tmp.path()) }; | ||
| (tmp, guard) |
There was a problem hiding this comment.
Suggestion: unset the env var when the guard drops so future tests that don't call with_temp_config_dir() don't accidentally inherit a stale (possibly deleted) path. One pattern is to return a custom RAII guard that removes the var on Drop instead of returning the MutexGuard directly.
Non-blocking — all current tests that touch disk already use this helper.
| files: lcov.info | ||
| fail_ci_if_error: false | ||
| env: | ||
| CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} |
There was a problem hiding this comment.
Consider pinning to a commit SHA instead of a mutable tag to guard against tag-movement supply chain risk, e.g. codecov/codecov-action@13afa45a3c5ef97eb2c28e9e3f20c3741badc8f0 # v4.
Non-blocking — fail_ci_if_error: false already limits the blast radius.
1 participant
No description provided.