Closed MitchellBerend closed 2 years ago
I must have clicked a ready button or something while reading some comments on this pr. It still needs a lot of work and I would also like to add a doc tests for the output of the default-config
command
Im adding some more tests to increase coverage. Im using cargo tarpaulin to check this. There are still some false negatives in tarpaulin but adding more tests is probably not a bad thing.
I have found that this method was not testable since it exits the program: https://github.com/chshersh/tool-sync/blob/4341cc734142fda35caacdd5c02d778d00698144/src/config/schema.rs#L45-L63
Sep 11 17:10:36.148 INFO cargo_tarpaulin::process_handling::linux: Launching test
Sep 11 17:10:36.148 INFO cargo_tarpaulin::process_handling: running /home/mitchell/rust/tool-sync/target/debug/deps/tool-711d73e056f99e07
running 0 tests
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
Sep 11 17:10:38.071 INFO cargo_tarpaulin::process_handling::linux: Launching test
Sep 11 17:10:38.071 INFO cargo_tarpaulin::process_handling: running /home/mitchell/rust/tool-sync/target/debug/deps/tool_sync-fd5c4dc2ba40d47f
running 21 tests
test config::toml::tests::empty_file ... ok
test config::toml::tests::store_directory_is_a_number ... ok
test config::toml::tests::only_store_directory ... ok
test config::toml::tests::single_empty_tool ... ok
test config::toml::tests::single_partial_tool ... ok
test config::toml::tests::single_full_tool ... ok
test config::toml::tests::store_directory_is_dotted ... ok
test config::toml::tests::two_empty_tools ... ok
test model::asset_name::tests::exe_name ... ok
test model::asset_name::tests::asset_name ... ok
test sync::configure::tests::full_override ... ok
test sync::configure::tests::full_configuration ... ok
test sync::configure::tests::known_tool_with_empty_config_asset ... ok
test sync::configure::tests::unknown_tool_with_empty_config_asset ... ok
test sync::configure::tests::partial_configuration ... ok
test sync::configure::tests::wrong_tool_with_empty_config_asset ... ok
test sync::download::tests::release_url_with_latest_tag_is_correct ... ok
test sync::configure::tests::partial_override ... ok
test sync::download::tests::release_url_with_specific_tag_is_correct ... ok
test sync::progress::tests::test_max_tag_size_specific ... ok
test sync::progress::tests::test_max_tag_size_latest ... ok
test result: ok. 21 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.05s
Sep 11 17:10:40.028 INFO cargo_tarpaulin::report: Coverage Results:
|| Uncovered Lines:
|| src/config/schema.rs: 45-46, 48-50, 53, 55-56, 58
|| src/config/toml.rs: 17-21, 26-27, 29
|| src/err.rs: 4-5, 12, 16-17, 30
|| src/lib.rs: 16-18, 20-26, 29-30, 33, 37-45, 47-48, 54-55
|| src/main.rs: 2
|| src/model/tool.rs: 19-22, 24
|| src/sync/archive.rs: 28-33, 40, 46, 48-53, 55-56, 58-63, 65-66, 68-72, 81-82, 84, 87-89, 93-95, 101, 103-106, 109-110, 112, 114, 117, 123, 126, 128-130, 133-134, 138, 140, 145-146, 148-151
|| src/sync/configure.rs: 22
|| src/sync/db.rs: 8-14, 16, 19-25, 27, 30-36, 38, 41-47, 49, 63-69, 71
|| src/sync/download.rs: 31-33, 37-38, 40-42, 46-47, 50-52, 55, 57, 60-61, 64-66, 69, 71-72, 74-75, 77-80, 83-84, 87-88, 90, 94-95, 97, 99-100, 102, 104-107, 109-112, 119-122
|| src/sync/install.rs: 29-33, 43-45, 47-50, 52-54, 57-59, 64, 69, 71, 73, 75-78, 83, 87-89, 92, 94, 96-100, 109-110, 112-114, 117, 119, 121, 128-129
|| src/sync/progress.rs: 38-39, 47, 50-51, 53-56, 60-61, 64-65, 68-69, 72-73, 75-77, 80-81, 83-85
|| src/sync.rs: 12-14, 32, 34-35, 38, 40-41, 43-44
|| Tested/Total Lines:
|| src/config/schema.rs: 0/9 +0.00%
|| src/config/toml.rs: 71/79 -10.13%
|| src/err.rs: 0/6 +0.00%
|| src/lib.rs: 0/26 +0.00%
|| src/main.rs: 1/2 +0.00%
|| src/model/asset_name.rs: 16/16 +0.00%
|| src/model/tool.rs: 4/9 +0.00%
|| src/sync/archive.rs: 0/63 +0.00%
|| src/sync/configure.rs: 108/109 +0.00%
|| src/sync/db.rs: 16/56 -71.43%
|| src/sync/download.rs: 14/66 +0.00%
|| src/sync/install.rs: 0/47 +0.00%
|| src/sync/progress.rs: 29/54 +0.00%
|| src/sync.rs: 0/11 +0.00%
||
46.84% coverage, 259/553 lines covered, -12.641681145031853% change in coverage
Sep 11 17:15:10.011 INFO cargo_tarpaulin::process_handling::linux: Launching test
Sep 11 17:15:10.011 INFO cargo_tarpaulin::process_handling: running /home/mitchell/rust/tool-sync/target/debug/deps/tool-711d73e056f99e07
running 0 tests
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
Sep 11 17:15:11.899 INFO cargo_tarpaulin::process_handling::linux: Launching test
Sep 11 17:15:11.899 INFO cargo_tarpaulin::process_handling: running /home/mitchell/rust/tool-sync/target/debug/deps/tool_sync-fd5c4dc2ba40d47f
running 27 tests
test config::toml::tests::store_directory_is_a_number ... ok
test config::toml::tests::empty_file ... ok
test config::toml::tests::only_store_directory ... ok
test config::toml::tests::single_empty_tool ... ok
test config::toml::tests::single_partial_tool ... ok
test config::toml::tests::single_full_tool ... ok
test config::toml::tests::test_toml_error_display_decode ... ok
test config::toml::tests::test_toml_error_display_io ... ok
test config::toml::tests::test_parse_file_error ... ok
test config::toml::tests::test_parse_file_correct_output ... ok
test config::toml::tests::store_directory_is_dotted ... ok
test config::toml::tests::test_parse_file_passing_tools_read ... ok
test config::toml::tests::two_empty_tools ... ok
test model::asset_name::tests::exe_name ... ok
test config::toml::tests::test_toml_error_display_parse ... ok
test model::asset_name::tests::asset_name ... ok
test sync::configure::tests::unknown_tool_with_empty_config_asset ... ok
test sync::configure::tests::partial_configuration ... ok
test sync::configure::tests::known_tool_with_empty_config_asset ... ok
test sync::configure::tests::partial_override ... ok
test sync::configure::tests::full_override ... ok
test sync::configure::tests::full_configuration ... ok
test sync::configure::tests::wrong_tool_with_empty_config_asset ... ok
test sync::progress::tests::test_max_tag_size_specific ... ok
test sync::progress::tests::test_max_tag_size_latest ... ok
test sync::download::tests::release_url_with_specific_tag_is_correct ... ok
test sync::download::tests::release_url_with_latest_tag_is_correct ... ok
test result: ok. 27 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.05s
Sep 11 17:15:13.851 INFO cargo_tarpaulin::report: Coverage Results:
|| Uncovered Lines:
|| src/config/schema.rs: 45-46, 48-50, 53, 55-56, 58
|| src/config/template.rs: 3-4
|| src/err.rs: 4-5, 12, 16-17, 30
|| src/lib.rs: 16-18, 20-26, 29-30, 33, 37-45, 47-48, 54-55
|| src/main.rs: 2
|| src/model/tool.rs: 19-22, 24
|| src/sync/archive.rs: 28-33, 40, 46, 48-53, 55-56, 58-63, 65-66, 68-72, 81-82, 84, 87-89, 93-95, 101, 103-106, 109-110, 112, 114, 117, 123, 126, 128-130, 133-134, 138, 140, 145-146, 148-151
|| src/sync/configure.rs: 22
|| src/sync/download.rs: 31-33, 37-38, 40-42, 46-47, 50-52, 55, 57, 60-61, 64-66, 69, 71-72, 74-75, 77-80, 83-84, 87-88, 90, 94-95, 97, 99-100, 102, 104-107, 109-112, 119-122
|| src/sync/install.rs: 29-33, 43-45, 47-50, 52-54, 57-59, 64, 69, 71, 73, 75-78, 83, 87-89, 92, 94, 96-100, 109-110, 112-114, 117, 119, 121, 128-129
|| src/sync/progress.rs: 38-39, 47, 50-51, 53-56, 60-61, 64-65, 68-69, 72-73, 75-77, 80-81, 83-85
|| src/sync.rs: 12-14, 32, 34-35, 38, 40-41, 43-44
|| Tested/Total Lines:
|| src/config/schema.rs: 0/9 +0.00%
|| src/config/template.rs: 0/2
|| src/config/toml.rs: 117/117 +10.13%
|| src/err.rs: 0/6 +0.00%
|| src/lib.rs: 0/26 +0.00%
|| src/main.rs: 1/2 +0.00%
|| src/model/asset_name.rs: 16/16 +0.00%
|| src/model/tool.rs: 4/9 +0.00%
|| src/sync/archive.rs: 0/63 +0.00%
|| src/sync/configure.rs: 108/109 +0.00%
|| src/sync/db.rs: 73/73 +71.43%
|| src/sync/download.rs: 14/66 +0.00%
|| src/sync/install.rs: 0/47 +0.00%
|| src/sync/progress.rs: 29/54 +0.00%
|| src/sync.rs: 0/11 +0.00%
||
59.34% coverage, 362/610 lines covered, +12.508819257107277% change in coverage
@chshersh Should the default tools also be generated in the default-config
command? This would make it so maintenance only needs to happen in one place (the BTreeMap
).
@MitchellBerend cargo tarpaulin
looks like a nice tool 🙂 I wouldn't try to gamify code coverage too much. Achieving 100% actually might be counterproductive as your test freeze your code too much and you need to do more and more to change the tests each time you change the code. At this stage, tool-sync
is quite unstable and lots of refactorings could happen by changing the codebase significantly.
It's useful however to identify important code paths that are not covered 👍🏻
Should the default tools also be generated in the default-config command? This would make it so maintenance only needs to happen in one place (the BTreeMap).
Yes, that was the plan. If you're up to it, you can go ahead and do this change in this PR (or leave it for later).
I think this is ready for review. I would like to have more tests still though but I agree with the not having too high of a code coverage. The tests I added were to confirm the formatting of printing errors when processing toml files so I would like those in.
Resolves #73 This adds a build script that generates the
src/config/template.rs
file. It is done this way because there is no access to version information after compile time. It uses theCARGO_PKG_VERSION
environment variable made available for cargo after running the commandcargo build
.Additional tasks
tasks following #73
General tasks