#364 split on

[MNie]

Related Issue: #364

I added a new option to config with a default value of /. @haf There is a single place that still uses a hardcoded '/' (Expecto.fs line 60). Should we care about that?

[haf]

Yes, we need to care about that line, too, if we're to make this refactor.

The alternative refactor is to represent test names as string arrays?

[MNie]

@haf , yup, I will update a pr tomorrow.

[MNie]

@haf I change a label from string to string list. But there are a couple of questions.

• what with all those helpers like testcasewithcancel etc, Do we want to accept string there and then transform it to a list?
• what with filtering, we want to filter based on all names, on the first part of a name, last part?
• getting the last element in a list via List.rev |> List.head isn't the best solution, maybe you have some better idea? Array instead of a list? Or something else?

I didn't adjust tests, because I want to do a checkpoint if this is what we want to achieve?

[haf]

what with all those helpers like testcasewithcancel etc, Do we want to accept string there and then transform it to a list?

The behaviour from the CLI should be identical.

what with filtering, we want to filter based on all names, on the first part of a name, last part?

Same here. So you'd have to parse the string and/or join the test name. Perhaps use a value object and memoize the construction of the joined name?

getting the last element in a list via List.rev |> List.head isn't the best solution, maybe you have some better idea? Array instead of a list? Or something else?

Never use head, let's see how that turns out. Also, see if you can avoid pattern matching by only using the list when needed rather than for all labels?

[haf]

BTW; this is a MAJOR version change. So I would like to have at least two other people who use Expecto testing this once tests are passing and it's stable.

[haf]

Also thank you for refactoring this

[MNie]

@haf I fix'd a couple of things. If you would have a couple of minutes to take a look and give some feedback it would be great! ;)

[teo-tsirpanis]

A better idea would have been to split the test names with dots, by default (for better Visual Studio experience and because of major release), and expose a --join-by-slash option to revert to the old behavior.

The way --join-by is formatted might mislead the user into thinking that any string can be used to split the test names (flexible, but there is no reason to me that liberal with regards to configuration).

[MNie]

@teo-tsirpanis I could change it to . as a default. Regarding the --join-by vs --join-by-slash from my point of view they are "equal". So if @haf is okay with that parameter name change I could also do that :)

[haf]

Yes I'm fine with that suggestion, to change it to dot and add the 'revert to slash' flag. I wanted a second pair of eyes on this, so thanks @teo-tsirpanis .

Could you do this and rebase as well and we can get this merged into the upcoming major bump?

[MNie]

@haf Yes, sure. I will do that today/over the weekend.

[haf]

Thank you @MNie ! Great job!