-
Notifications
You must be signed in to change notification settings - Fork 379
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 support for "pinned" tests #2803
Conversation
9234823
to
2973e19
Compare
Heh, I was so used to the old procedure that the idea of somehow improving it never crossed my mind 😅 "static" as a target name is problematic though, "static" in the build system context usually refers to static linkage. Maybe "pinned", or as per the the README, or "check-pinned" to make it clear it relates to tests? |
Yup, I knew the "static" name probably wasn't great because of what you said but also because those tests aren't really "static" any more than the normal tests, actually quite the opposite 😄 For some reason, "pinned" doesn't sound very descriptive either, though. And "check-pinned" even less so... Also, in that case we should probably move all the test-related targets to the "check-" prefix (which gets unwieldy pretty quickly 😄 ). But "static" is a no-go, I agree. I'll give it a quick thought and do a fixup. Thanks! |
Pinned is not really descriptive but at least it wont be confused with something else in the build system domain. But yup, perhaps we can come up with something actually descriptive. Names are hard 😆 |
For other random ideas, perhaps something better would be in the direction of what it actually does, eg "version-diff", or what it relates to: preparing for release (release-prep) or such. |
"version-diff" indeed is perhaps closer to what it does, however it just happens that these tests depend on the version. There might as well be other tests (in the future) that depend on some other baked-in value. "release-prep" is similarly too generic because this really is just about the tests, not necessarily about an imminent release (e.g. we need to "pin" these tests even if we bump the micro version to .99 on master). But yup, I was too thinking of having a "release" target that would automate some of the release prep work, in which case this target would be made a dependency of it. Other ideas:
Edit: "reproducible-tests" would also be a bit too specific (to that one test) but the general idea perhaps is along the same lines for all such tests... not sure. |
The idea behind this kind of tests is basically this: We need to track the expected output in the repository so that we ensure it's always the same when running the given test, which is why we don't just dynamically determine the expected output at test-runtime. At the same time, we need this output to be easily regenerated based on the current repo content. |
... which makes me think about another target name: update-tests 😄 |
And perhaps, if we were to put all the test-related targets behind the "check-" prefix, then "check-update"... (apt-get anyone? 😄 ) |
Leafing through the mental dictionary of handy words... Mutable! It fits the bill pretty perfectly as these results are expected to change at certain times, whereas all the other tests do not. So perhaps "update-mutable"? You said it yourself: these are not static but quite the opposite. |
Oh! That's a pretty good idea! 😄 ... except that "mutable" is already used in the test context. But - that doesn't matter much, I suppose. It's a lower-level detail that's relevant when writing a test. Otherwise, "mutable" is indeed the best fit. So, let's just go with that. |
Yeah, mutable and immutable are generic concepts and used in multiple places already. So if you really want to pinpoint it, "update-mutable-tests" 😄 (but of course that's already annoyingly long) |
Yup, annoyingly as well as amusingly long 😄 But as a rule of thumb, naming make targets after the "artifacts" they produce rather than the "action" they do is always better, at least IMO (of course we don't follow that precisely but that's fine). |
Whenever we bump the RPM version in cmake, certain tests also need updating, e.g. those that compare package digests or RPM sonames as those are subject to the version number. Up until now, this had to be done manually by running the test-suite, looking at the failed tests' diffs and fixing up the expected outputs by hand. No more! Separate those tests and add the "make pinned" cmake target to update them automatically. This makes use of the new-ish atshell feature which runs a shell/command (*.sh scripts in tests/pinned/ in this case) in the standard test environment in order to get the new outputs. See the included README for more details.
OK, in the end I went with simply |
I also updated the README to make it a bit more clear. |
Three weeks should be enough think-time for this and since I didn't come up with anything... |
Thanks 😄 It's basically a private target only used by a handful of people so we have some leeway in terms of choosing a better name and/or implementation of this in the future, but for now, this should suffice. |
Whenever we bump the RPM version in cmake, certain tests also need updating, e.g. those that compare package digests or RPM sonames as those are subject to the version number.
Up until now, this had to be done manually by running the test-suite, looking at the failed tests' diffs and fixing up the expected outputs by hand.
No more! Separate those tests and add the "make pinned" cmake target to update them automatically. This makes use of the new-ish atshell feature which runs a shell/command (*.sh scripts in tests/pinned/ in this case) in the standard test environment in order to get the new outputs.
See the included README for more details.