Consider renaming `testdata.yaml` to `test_group.yaml`

[mpsijm]

In a discussion with @thorehusfeldt, we stumbled upon the confusion between test data, which is a more abstract term (in my eyes the collection of groups and cases), and test groups. BAPCtools still needs to improve the distinction between testdata.yaml for test groups and <test_case>.yaml for test cases (see also the TODO-list at RagnarGrootKoerkamp/BAPCtools#433), and in doing so, we got the idea of renaming testdata.yaml to test_group.yaml to make the distinction with a <test_case>.yaml clearer. What do others think of this?

Note that this would also mean that, for most standard floating-point output problems, there will be a single data/test_group.yaml file with output_validator_args set to some float tolerance value. If we see data/ as one big test group, I think that's fine.

If people don't agree with the name test_group.yaml, we can also rename to test_data.yaml, to slightly improve the grammar-correctness of the name of this file.

[evouga]

Sure, I like test_group.yaml.

[niemela]

I don't really see why changing the name make the distinction with <test case>.yaml any clearer? It's still true (and annoying) that one special name (not testdata, if we do this change test_group) is treated differently, and thus is not allowed as a name of a test case.

The testdata.yaml originally specifies how the results of the test data group should be calculated, so it would make a lot of sense if some combination of the words "result", "test", "data", "group" (or their synonyms) are used to name this file. That said, we have added some fields that specify different metadata for the test cases in the group... so it's arguably more than "result" now, so we probably shouldn't use that word.

I do think that "test group" is better than "test data" for this file... but they are both relevant and "right". Is it worth it to make the change though? OTOH, I think we have changed the contents of the file so much that no testdata.yaml from the legacy format would still be correct, right?

One argument against the name change is that many (most?) think of the groups as only those under /secret, and this would apply also for problems with only /sample and /secret, i.e. problems that "don't use groups". I'm not sure how good that argument is?

[evouga]

OTOH, I think we have changed the contents of the file so much that no testdata.yaml from the legacy format would still be correct, right?

A legacy file with only input or output validator flags should still be valid. I don't think the ICPC subset allows testdata.yaml at all.

But I don't think that's a strong argument for keeping the name: if we agree that test_group is clearer, it's less disruptive to change the name now than to change it later.

I do think "test group" is clearer than "testdata", but don't feel strongly.

[niemela]

A legacy file with only input or output validator flags should still be valid. I don't think the ICPC subset allows testdata.yaml at all.

Yeah, that's right.

But I don't think that's a strong argument for keeping the name: if we agree that test_group is clearer, it's less disruptive to change the name now than to change it later.

I do think "test group" is clearer than "testdata", but don't feel strongly.

I'm kinda leaning the same way... but it's not much better.

Again, this is a metadata file that sets metadata for the test cases and subgroups in this group... Myabe just group.yaml would be better?

[thorehusfeldt]

I do like group.yaml, but a problem with too short or unspecific name is that it clashes with a test case configuration file in the same directory. (Recall that test case secret/group would have its input in secret/group.in and its configuration in secret/group.yaml. The same argument would be levelled at another attractive name: config.)

This is a weak argument for using a very specific name, such as test_group_config.yaml.

But “group”-ishness (in contrast with “case”-ishness) is the most important concept to signal. (And testdata.yaml fails to do that.) I remain weakly in favour of test_group.yaml.

[niemela]

I do like group.yaml, but a problem with too short or unspecific name is that it clashes with a test case configuration file in the same directory. (Recall that test case secret/group would have its input in secret/group.in and its configuration in secret/group.yaml. The same argument would be levelled at another attractive name: config.)

Agreed. The shorter the name, the worse that problem becomes.

How about group_config.yaml?

This is a weak argument for using a very specific name, such as test_group_config.yaml.

Yes, now when we share namespace... a worse name is also better.

But “group”-ishness (in contrast with “case”-ishness) is the most important concept to signal.

But is it actually? (I mean I think so, but...). Several of the settings are really setting for the cases, not the group. Now, again, they are setting for all cases in the group.

(And testdata.yaml fails to do that.) I remain weakly in favour of test_group.yaml.

[mpsijm]

Regarding the name clash: most test cases for the problems that I work with, are prefixed with numbers. (BAPCtools' generators framework does this automatically when specifying the test cases as a list in generators.yaml.) So at least there, the name clash does not happen 🙂

I like both test_group.yaml and group_config.yaml, I think both are roughly equally likely to cause a name clash with unnumbered test cases. test_group_config.yaml is too long to my liking, but that's subjective.

Also note that we already have the possibility of a name clash with the existing testdata.yaml, so we should simply write in the spec that a test case cannot be named exactly the same as the name as the group's config file (whichever one we choose), so tooling can enforce this.

One argument against the name change is that many (most?) think of the groups as only those under /secret, and this would apply also for problems with only /sample and /secret, i.e. problems that "don't use groups". I'm not sure how good that argument is?

We could also use this argument the other way around. If we write the spec in such a way that we describe /data as one top-level group, and /data/{sample,secret} as sub-groups, then it should be fine to use "group" in the name of its configuration file. The alternative would be to have both /data/test_data.yaml and /data/secret/test_group.yaml, for instance, but that would also cause confusion so then I prefer to keep one consistent name 😛

[niemela]

Regarding the name clash: most test cases for the problems that I work with, are prefixed with numbers. (BAPCtools' generators framework does this automatically when specifying the test cases as a list in generators.yaml.) So at least there, the name clash does not happen 🙂

Agreed, most packages I see follow this (and I would argue that it is very good practice to follow this, to the point that I would almost want to require it)

I like both test_group.yaml and group_config.yaml, I think both are roughly equally likely to cause a name clash with unnumbered test cases. test_group_config.yaml is too long to my liking, but that's subjective.

Yes, my subjective opinion is similar.

Also note that we already have the possibility of a name clash with the existing testdata.yaml, so we should simply write in the spec that a test case cannot be named exactly the same as the name as the group's config file (whichever one we choose), so tooling can enforce this.

We already do. testdata is currently an illegal test case name. (It's at the end of the second paragraph of the test data section).

One argument against the name change is that many (most?) think of the groups as only those under /secret, and this would apply also for problems with only /sample and /secret, i.e. problems that "don't use groups". I'm not sure how good that argument is?

We could also use this argument the other way around. If we write the spec in such a way that we describe /data as one top-level group, and /data/{sample,secret} as sub-groups, then it should be fine to use "group" in the name of its configuration file.

Yeah, that's a good point!

The alternative would be to have both /data/test_data.yaml and /data/secret/test_group.yaml, for instance, but that would also cause confusion so then I prefer to keep one consistent name 😛

No, I agree, that's terrible.