-
-
Notifications
You must be signed in to change notification settings - Fork 209
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add an id
property to all test cases
#698
Comments
If you'd like to assign me to this, I'll take it on. I propose the following in response to this:
|
I originally presumed we would have some sort of human readable ID. UUIDs would be simpler in some respects. If someone has a list of tests they skip, having human readable names which partially suggest the sort of test in question, it could be helpful. However, this wouldn't be the best if we agree that IDs should change if the test changes (which I think is a good idea). In that case, I wonder if a simpler approach is to just MD5 the test object... although I suspect because of JSON and object ordering, that might not end up the same depending on various things. All that to say, are we convinced UUIDs as IDs would be the best here? |
I did consider an MD5 hash of the test object (and the schema in the case of the test case) but that makes it rather harder to generate while developing the tests, and feels like a barrier to entry. |
I don't really have any strong preference on the structure of the IDs -- I think I mentioned I think slugs are better because of the extra benefit (of being user readable and usable for other reasons in isolation) but if I'm not doing the work I definitely don't care. The only thing I do care about is:
I don't personally want to (ever) review large automated PRs (and have said elsewhere that as far as I have seen in "real life" it's simply impossible for any human to do.) -- meaning I'm happy to review the script you write to do this, or else for someone else who thinks it's possible to do the review -- though experience in this repo itself has reaffirmed as far as I'm concerned that this is the case :) -- the last two times I said this (no large automated PRs) and others reviewed it, the PRs indeed introduced subtle test bugs that were fixed over a few months afterwards (not that it took months to fix, it took months to notice, which is often the problem). So yeah, happy to review the script, or else to say "find any other test suite contributor willing to review the PR". Otherwise I'm fine with UUIDs if you prefer. |
Problem: Tests in the suite are occasionally moved around in the file, or their descriptions are clarified, or they are modified because they have a bug. Given that the intended use of the test suite is that downstream users -- (all implementations, Bowtie, other research use cases) -- be able to run the tests, they do not have a way to "track" a test over time. Specifically, say an implementation has a known issue with a specific test for
$dynamicRef
-- a test case called$dynamicRef flubs the bub when you bla bla
. An implementer who wishes to mark this test as a TODO in their test suite has no reliable way to do so over the long term, because it's name, position within the file, or even file itself may change here upstream for various reasons.We should therefore introduce a stable ID which we guarantee will never change for a test, even if its description changes, or if the test is moved elsewhere within the file or suite.
A "slug" is likely the simplest choice structure-wise, in that it's derivable from the existing test descriptions. UUIDs or globally incrementing integers are other options (see below for considerations).
This addition should also include a sanity check ensuring that the IDs have the properties we wish of them (i.e. uniqueness, see below).
For context, we had previously added this exact concept to the test case schema in a PR #53 but did not ever add it to tests
Considerations:
(version, ID)
.The text was updated successfully, but these errors were encountered: