Skip to content

Conversation

@lzrd
Copy link

@lzrd lzrd commented Dec 9, 2025

Several tests were failing. It looks like code had changed from underneath them.
There were also some "complex enough" code paths that were not being tested.

This also includes clippy fixes within tests that were not caught as part of the previous clippy PR.

lzrd and others added 7 commits December 9, 2025 12:21
Mostly mechanical fixes: removing needless borrows, adding lifetime
annotations, using modern numeric constants (i32::MAX), and suppressing
warnings for dropshot-generated dead code.

Notable equivalences:
- .as_bytes().len() to .len() on String: both return byte count
- .trim().split_whitespace() to .split_whitespace(): split_whitespace
  already skips leading/trailing whitespace
- .into_iter() to .iter() on slices: both yield &T
Co-authored-by: Joshua M. Clulow <[email protected]>
Co-authored-by: Joshua M. Clulow <[email protected]>
Co-authored-by: Joshua M. Clulow <[email protected]>
Co-authored-by: Joshua M. Clulow <[email protected]>
lzrd added 3 commits December 10, 2025 10:24
Tests in github/testdata and github/server were using
buildomat_github_database::types::JobFile which lacks the
parse_content_at_path method. Switch to buildomat_jobsh::jobfile::JobFile
which has the parsing functionality needed by the tests.
jobsh/src/jobfile.rs:
  Add 16 tests for JobFileSet and JobFile parsing. The JobFileSet
  struct has complex logic for duplicate name detection, dependency
  cycle detection, and reference validation that was previously
  untested.

github/database/src/tables/check_run.rs:
  Add 4 tests for CheckRun::get_dependencies(). This method
  deserializes JSON dependency data and was not covered by existing
  enum serialization tests.

github/database/src/tables/user.rs:
  Add 2 tests for UserType::from_github_str(). The error path for
  invalid GitHub API strings was untested.

types/src/config.rs:
  Add 2 tests for ConfigFileDiag inheritance. The explicit
  "inherit: true" case and mixed inheritance scenarios were not
  covered by existing tests.
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.

2 participants