Test the extended configuration part in the 'default-config' output

[chshersh]

The default configuration provides an example of a more detailed config:

https://github.com/chshersh/tool-sync/blob/df7d4768c9cff2a91616bd9686bc928a52fc8c10/tests/default-config.toml#L22-L38

However, this is not tested at all. The TOML format might diverge and there's no way to know if we need to change the default config. So I propose to automate testing by doing the following:

• [ ] Implement to TOML serialization for ToolInfo
• [ ] Write a property-based test that generated random ToolInfo and makes sure that decoding and encoding satisfy the roundtrip property
• [ ] Generate the default config from a hardcoded ToolInfo without adding comments for the expanded version
• [ ] Make sure that it parses and returns our hardcoded ToolInfo
• [ ] Add comments for the generated config in the corresponding lines

[MitchellBerend]

Would the implementation of this serialization be implemented in the binary itself or only in the test?

Another question I have is do we add fuzzing for this test? This could potentially be flaky but I like the idea of testing random inputs.

[chshersh]

Would the implementation of this serialization be implemented in the binary itself or only in the test?

It'll be used only for tests. So I expect that it'll be stripped as an unused symbol (our current release process runs strip). But even if not, it's just a small part, so I'm not worried too much.

I propose to implement it in the binary. We might want to add a feature where we suggest putting the full table in the config, so this serialization might be handy.

Another question I have is do we add fuzzing for this test? This could potentially be flaky but I like the idea of testing random inputs.

Fuzzing would be nice. All fields in the ConfigAsset are optional. So, ideally, we want to test that they are parsed correctly. We have lots of fields so testing them manually leads to combinatorial explosion. That's why I proposed roundtrip property-based tests. But it can be done separately, no need to insert a random tool inside the default config. It'll complicate things without much benefit.

[MitchellBerend]

I have been thinking about this and what are we actually testing here? Initially I thought it was about the conversion to and from toml being commutative but reading more into this, that would mean the comments aren't of consequence.

Is the intent here just to check if the tests/default-config.toml is correct?

[chshersh]

@MitchellBerend Indeed, it's always good to revisit our assumptions 😌

In this issue, I want to only test the default-config.toml and make sure that the code under comments is valid. The way I imagined that is to implementing an encoding function for ToolInfo so it can be used in the generation of the default config. And since we have two functions (for decoding and encoding) it makes sense to test that they are consistent with each other.

But after checking the issue and conversation again, I've realised two things:

1. The default config has comments for each field. We can't do this in the encoding function.
2. The decoding function returns ConfigAsset while we're encoding ToolInfo. These are different types.

So yeah, we don't really need fuzzy testing. We just need to create a single const value that we're going to use in the default config and just format it in two ways: commented and uncommented (so we can test the decoding).

Maybe you can even use StaticToolInfo to define the hardcoded example easier 🤔