Skip to content

chore: Update clippy rules and add test cases to increase code coverage#45

Merged
Artifizer merged 1 commit intoGlobalTypeSystem:mainfrom
joseph-cx:chore/clippy-and-coverage
Feb 3, 2026
Merged

chore: Update clippy rules and add test cases to increase code coverage#45
Artifizer merged 1 commit intoGlobalTypeSystem:mainfrom
joseph-cx:chore/clippy-and-coverage

Conversation

@joseph-cx
Copy link
Copy Markdown
Contributor

@joseph-cx joseph-cx commented Jan 29, 2026

  1. Update clippy rules to match hyperspot/cyber fabric repo.
  2. Added unit tests to all files in gts library
  3. Added additional unit tests to gen_schemas.rs
  4. Added integration tests for cli.rs, logging.rs, server.rs in gts-cli binary crate
  5. Added additional compile_fail and integration tests for gts-macros/src/lib.rs
  6. Overall code coverage increase: 81.46% → 94.55% (+13.09%)

@joseph-cx joseph-cx force-pushed the chore/clippy-and-coverage branch 5 times, most recently from b5ad59c to c2722e4 Compare January 29, 2026 06:04
@joseph-cx
Copy link
Copy Markdown
Contributor Author

joseph-cx commented Jan 29, 2026

CI for windows is failing due to new tests cases in gts-macros-cli/src/main.rs. In save_schema, the function tries to save a schema to a file, with the file name being: {schema_id}.schema.json. From what I can tell, the problem is that, schema_id can have characters like ~ or : which are not valid characters for windows files.

Will remove the failing test case for now.

@joseph-cx joseph-cx linked an issue Jan 29, 2026 that may be closed by this pull request
3 tasks
@joseph-cx joseph-cx force-pushed the chore/clippy-and-coverage branch 4 times, most recently from 5826ea5 to c2a23b3 Compare January 30, 2026 03:09
@joseph-cx joseph-cx requested a review from Artifizer January 30, 2026 03:12
@joseph-cx
Copy link
Copy Markdown
Contributor Author

Code Coverage Increases

File-by-File Coverage Improvements

File Previous Coverage Current Coverage Increase
gts-cli/src/cli.rs 0.00% 90.60% +90.60%
gts-cli/src/gen_schemas.rs 47.19% 93.81% +46.62%
gts-cli/src/logging.rs 0.00% 93.98% +93.98%
gts-cli/src/main.rs 0.00% 0.00% 0.00%
gts-cli/src/server.rs 0.00% 83.87% +83.87%
gts-macros-cli/src/main.rs 0.00% 88.42% +88.42%
gts-macros/src/lib.rs 78.24% 87.43% +9.19%
gts/src/entities.rs 95.74% 96.26% +0.52%
gts/src/files_reader.rs 21.83% 97.99% +76.16%
gts/src/gts.rs 85.04% 89.74% +4.70%
gts/src/ops.rs 91.00% 98.35% +7.35%
gts/src/path_resolver.rs 85.71% 85.71% 0.00%
gts/src/schema.rs 30.53% 93.80% +63.27%
gts/src/schema_cast.rs 93.22% 93.22% 0.00%
gts/src/store.rs 94.21% 96.34% +2.13%
gts/src/x_gts_ref.rs 73.13% 98.70% +25.57%

Overall Coverage Increase

Total Line Coverage: 81.46% → 94.55% (+13.09%)

Comment thread gts/src/gts.rs
#[derive(Debug, Error)]
pub enum GtsError {
#[error("Invalid GTS segment #{num} @ offset {offset}: '{segment}': {cause}")]
InvalidSegment {
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.

I believe we can't rename enum variants in put GtsError because it would be incompatible change, isn't it?

Copy link
Copy Markdown
Contributor Author

@joseph-cx joseph-cx Feb 2, 2026

Choose a reason for hiding this comment

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

This was renamed due to clippy error:

error: all variants have the same prefix: `Invalid`
  --> gts/src/gts.rs:31:1
   |
31 | / pub enum GtsError {
32 | |     #[error("Invalid GTS segment #{num} @ offset {offset}: '{segment}': {cause}")]
33 | |     InvalidSegment {
34 | |         num: usize,
...  |
44 | |     InvalidWildcard { pattern: String, cause: String },
45 | | }
   | |_^
   |
   = help: remove the prefixes and use full paths to the variants instead of glob imports
   = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.93.0/index.html#enum_variant_names
   = note: `-D clippy::enum-variant-names` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(clippy::enum_variant_names)]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I can revert the changes for this and suppress the error if you want. I think its not really a big deal for this enum since its just 3 variants.

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.

OK then

Comment thread gts/src/ops.rs

#[must_use]
pub fn validate_id(&self, gts_id: &str) -> GtsIdValidationResult {
pub fn validate_id(gts_id: &str) -> GtsIdValidationResult {
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.

This is a breaking change, would you please explain why it was needed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There are 2 reasons why this was removed:

  1. validate_id is a stateless function that does not require any instance states
  2. Clippy complained about unused self argument in this function

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.

OK

@Artifizer
Copy link
Copy Markdown
Contributor

Code Coverage Increases

File-by-File Coverage Improvements

File Previous Coverage Current Coverage Increase
gts-cli/src/cli.rs 0.00% 90.60% +90.60%
gts-cli/src/gen_schemas.rs 47.19% 93.81% +46.62%
gts-cli/src/logging.rs 0.00% 93.98% +93.98%
gts-cli/src/main.rs 0.00% 0.00% 0.00%
gts-cli/src/server.rs 0.00% 83.87% +83.87%
gts-macros-cli/src/main.rs 0.00% 88.42% +88.42%
gts-macros/src/lib.rs 78.24% 87.43% +9.19%
gts/src/entities.rs 95.74% 96.26% +0.52%
gts/src/files_reader.rs 21.83% 97.99% +76.16%
gts/src/gts.rs 85.04% 89.74% +4.70%
gts/src/ops.rs 91.00% 98.35% +7.35%
gts/src/path_resolver.rs 85.71% 85.71% 0.00%
gts/src/schema.rs 30.53% 93.80% +63.27%
gts/src/schema_cast.rs 93.22% 93.22% 0.00%
gts/src/store.rs 94.21% 96.34% +2.13%
gts/src/x_gts_ref.rs 73.13% 98.70% +25.57%

Overall Coverage Increase

Total Line Coverage: 81.46% → 94.55% (+13.09%)

This is great result!

@joseph-cx joseph-cx force-pushed the chore/clippy-and-coverage branch 4 times, most recently from c32cca9 to 2e9c484 Compare February 3, 2026 05:01
@joseph-cx joseph-cx force-pushed the chore/clippy-and-coverage branch from 2e9c484 to d83e31d Compare February 3, 2026 05:05
@joseph-cx joseph-cx requested a review from Artifizer February 3, 2026 06:45
Copy link
Copy Markdown
Contributor

CI for windows is failing due to new tests cases in gts-macros-cli/src/main.rs. In save_schema, the function tries to save a schema to a file, with the file name being: {schema_id}.schema.json. From what I can tell, the problem is that, schema_id can have characters like ~ or : which are not valid characters for windows files.

Will remove the failing test case for now.

Schema_id should not have ":", because this is prohibited symbol in GTS id.
The "~" exists, but AFAIK this is a valid symbol for Windows filesystems

Can you please investigate how to fix it?

@Artifizer
Copy link
Copy Markdown
Contributor

CI for windows is failing due to new tests cases in gts-macros-cli/src/main.rs. In save_schema, the function tries to save a schema to a file, with the file name being: {schema_id}.schema.json. From what I can tell, the problem is that, schema_id can have characters like ~ or : which are not valid characters for windows files.

Will remove the failing test case for now.

I create a new task for this:
#49

@Artifizer Artifizer merged commit b951f37 into GlobalTypeSystem:main Feb 3, 2026
7 checks passed
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.

gts-rust code cleanup and 95% code coverage

2 participants