Dived cairo-coverage to crates

[ksew1]

Stack:

• Delete cairo-coverage-args #123
• Add docs #121
• Add tests for clean subcommand #120
• Add clean subcommand #119
• Add run subcommand #118
• Dived cairo-coverage to crates #117 ⬅

⚠️ Part of a stack created by spr. Do not merge manually using the UI - doing so may have unexpected results.

[ksew1]

I think it's time to split cairo-coverage into multiple crates. If we add new subcommands (raw text coverage) in the future, it could become overwhelming. So, I propose the following division:

• cairo-coverage-core: The main coverage logic, structured as a library.
• cairo-coverage: A binary crate that handles the high-level orchestration and calls subcommands. The core coverage logic will be moved to cairo-coverage-core.
• cairo-coverage-args: Arguments and parsing logic for subcommands.
• tests-utils: Utilities and helpers for writing tests.

What do you think? Do you have any other ideas or suggestions in mind?

[THenry14]

I think it's time to split cairo-coverage into multiple crates. If we add new subcommands (raw text coverage) in the future, it could become overwhelming. So, I propose the following division:

* **`cairo-coverage-core`**: The main coverage logic, structured as a library.

* **`cairo-coverage`**: A binary crate that handles the high-level orchestration and calls subcommands. The core coverage logic will be moved to `cairo-coverage-core`.

* **`cairo-coverage-args`**: Arguments and parsing logic for subcommands.

* **`tests-utils`**: Utilities and helpers for writing tests.

What do you think? Do you have any other ideas or suggestions in mind?

This is a good idea, but I'd like to understand a bit more about this specific divide. As an example - what will be the gain of having cairo-coverage-args externalised? My understanding is that coverage is not used as a library anywhere (is there a plan for it to be that way?). Maybe cairo-coverage and cairo-coverage-args could be combined as one, since a binary crate probably requires the interface no matter what?

About test-utils, this seems like a very generic name, I'd be afraid that this could interfene with something tbh.
The idea itself could be interesting if the utils will be usable across all coverage crates. Can you shed some light on this as well?

thanks 🙏

[ksew1]

This is a good idea, but I'd like to understand a bit more about this specific divide. As an example - what will be the gain of having cairo-coverage-args externalised? My understanding is that coverage is not used as a library anywhere (is there a plan for it to be that way?). Maybe cairo-coverage and cairo-coverage-args could be combined as one, since a binary crate probably requires the interface no matter what?

About test-utils, this seems like a very generic name, I'd be afraid that this could interfene with something tbh. The idea itself could be interesting if the utils will be usable across all coverage crates. Can you shed some light on this as well?

thanks 🙏

The cairo-coverage-args crate was created because cairo-coverage-core requires RunArgs as input. Moving RunArgs to cairo-coverage would have introduced a cyclic dependency, as cairo-coverage depends on cairo-coverage-core, and cairo-coverage-core would then depend on RunArgs. By using a separate crate, this cyclic dependency is avoided.

Alternatively, this issue could have been resolved by creating a mirror struct of RunArgs within cairo-coverage-core.

As for test-utils, these were created because, later in this stack, they will be used in both cairo-coverage-core and cairo-coverage.

Regarding test-utils, this name seems very generic, and I’d be concerned that it could interfere with something.

Could you explain what you mean by this? 😅

[THenry14]

This is a good idea, but I'd like to understand a bit more about this specific divide. As an example - what will be the gain of having cairo-coverage-args externalised? My understanding is that coverage is not used as a library anywhere (is there a plan for it to be that way?). Maybe cairo-coverage and cairo-coverage-args could be combined as one, since a binary crate probably requires the interface no matter what?
About test-utils, this seems like a very generic name, I'd be afraid that this could interfene with something tbh. The idea itself could be interesting if the utils will be usable across all coverage crates. Can you shed some light on this as well?
thanks 🙏

The cairo-coverage-args crate was created because cairo-coverage-core requires RunArgs as input. Moving RunArgs to cairo-coverage would have introduced a cyclic dependency, as cairo-coverage depends on cairo-coverage-core, and cairo-coverage-core would then depend on RunArgs. By using a separate crate, this cyclic dependency is avoided.

Alternatively, this issue could have been resolved by creating a mirror struct of RunArgs within cairo-coverage-core.

As for test-utils, these were created because, later in this stack, they will be used in both cairo-coverage-core and cairo-coverage.

Regarding test-utils, this name seems very generic, and I’d be concerned that it could interfere with something.

Could you explain what you mean by this? 😅

Should core require external interface-things, especially if it is intended to be the de facto library? Maybe I'm not understanding something, but I think that if something is to be considered a library, it should probably be self-contained and not require externally imported interface to work 🤔 Maybe this (interface) should be a part of the library, and it should be imported in the binary crate from there as well? I don't think duplicating the code (mirror structs) is the most optimal here tbh.

As for test-utils, these were created because, later in this stack, they will be used in both cairo-coverage-core and cairo-coverage.

Sure, makes sense. Just to make sure though - what will end up there? MNost probably the core will have unit/integration tests, the the binary will have e2e right? Is there a code that will have to be shared between those two types of tests?

Could you explain what you mean by this? 😅

Sure, sorry if this was too ambiguous. I meant that test-utils name is too broad and generic in my opinion - there are already crates called test_utils, unit_test_utils etc on crates.io, which may lead to confusion (or worst case scenario - someone just makes a mistake and uses the wrong crate). I'd rather stay away from such names, and use some more delibarate ones like coverage-test-utils or something.

[ksew1]

Should core require external interface-things, especially if it is intended to be the de facto library? Maybe I'm not understanding something, but I think that if something is to be considered a library, it should probably be self-contained and not require externally imported interface to work 🤔 Maybe this (interface) should be a part of the library, and it should be imported in the binary crate from there as well? I don't think duplicating the code (mirror structs) is the most optimal here tbh.

Yep, you are right. The interface should probably also be there. But to be honest, I think duplicating the structs is the most optimal solution. A library shouldn’t dictate the interface of a binary, and vice versa. So, this should be separated, and I think duplicating the structs won’t cause any issues. It would actually provide more flexibility if I wanted to change something in the library while maintaining the binary interface.

Sure, makes sense. Just to make sure though - what will end up there? MNost probably the core will have unit/integration tests, the the binary will have e2e right? Is there a code that will have to be shared between those two types of tests?

Some utilities related to assert_fs, which are currently used in both the core and binary, are placed there for now. However, who knows what the future holds? 😵‍💫

Sure, sorry if this was too ambiguous. I meant that test-utils name is too broad and generic in my opinion - there are already crates called test_utils, unit_test_utils etc on crates.io, which may lead to confusion (or worst case scenario - someone just makes a mistake and uses the wrong crate). I'd rather stay away from such names, and use some more delibarate ones like coverage-test-utils or something.

Good catch. Maybe the name should be cairo-coverage-test-utils to have the same prefix as the other crates, maintaining consistency and code feng shui xd.

[THenry14]

Yep, you are right. The interface should probably also be there. But to be honest, I think duplicating the structs is the most optimal solution. A library shouldn’t dictate the interface of a binary, and vice versa. So, this should be separated, and I think duplicating the structs won’t cause any issues. It would actually provide more flexibility if I wanted to change something in the library while maintaining the binary interface.

Ok, I think I agree :D duplicating scares me a bit, but the way you've laid it makes sense. Let's make this, see how it looks like and make a final decision. Would that be ok?