User Details
- User Since
- Aug 2 2014, 8:08 AM (406 w, 4 d)
Sat, Apr 30
In retrospect, I should have removed/edited the metadata in the commit message. It looks nice on Phabricator, but it looks less nice in raw git/email form.
Apr 11 2022
Feb 24 2022
It feels so weird reading modern C++ again.
Feb 1 2022
This could also be handled in the Makefile to avoid having to dance around FreeBSD vs NetBSD in the test, since I assume we'll never upstream the utility name item and will always deviate from NetBSD.
Dec 22 2021
I understand and appreciate what you're striving for, but a major word of caution: tests that involve this degree of complexity have lower determinism, reducing the level of trust in a given test over the long run.
Nov 29 2021
Nov 28 2021
Tests look good! Thank you 💚!
Nov 9 2021
Also, this seems like (and I'm going to regret saying this) something that a system like autoconf might excel at (*shudders*).
There are/were other facilities in ATF that supported compile-time testing, but I think they should be removed, not expanded on.
Plus upstream has ../a.out: instead of awk: in the
output. Not sure how to deal with this yet, so I've not proposed
anything upstream.
I'm sorry for not responding sooner.. just one comment (otherwise, it looks great!).
Jul 9 2021
While here: have you investigated pytest? It’s far more extensible and pluggable than unittest.
This, seems like a bit of a misnomer. ATF integration isn’t done: instead unittest is being executed and faking metadata output to sort of act like ATF, but not completely. I would clarify this fact in the commit to avoid overselling what’s being provided.
Jun 14 2021
May 11 2021
These are a lot of changes to an upstream (NetBSD) provided file without good annotations (please look for "Begin FreeBSD" elsewhere).
Apr 21 2021
Apr 16 2021
Not my domain of expertise. Maybe someone like @dim or Steve Kargl would have better input 🤷...
Apr 14 2021
This needs to be run through rcorder to ensure things haven't been broken. Please add the output of this information to the CR.
Apr 12 2021
Mar 1 2021
Feb 26 2021
Feb 25 2021
Feb 24 2021
Is the affected_by_bug_208703 #define still relevant, i.e., should it be removed?
Feb 23 2021
Ah, nevermind. I didn't see the "Testing Done" section before leaving a comment.
This seems ok, but I'd like to see test results for the entire set of libm tests after this change.
Can you please post the test results to prove that this works now?
This conversion is on the right direction, but incomplete.
Feb 12 2021
Feb 4 2021
Oh, I see now based on the description.
The shellcheck warning fixes are great (I'll happily stamp that diff)!
Could you please wrap the changes in #ifdef __FreeBSD__ so it'd be easier for me to upstream to NetBSD?
I don't think this is a good idea. Tests really shouldn't be starting services because it can make the tests more flaky, it can mutate system state, and might interfere with other service management systems, like Isilon has on OneFS.
Feb 3 2021
This might be a good thing to add to a general purpose library.
I agree with @brooks ; the functionality that involves compiling as part of tests is very dubious functionality that should be removed.
Jan 28 2021
This isn't the right approach; please use getconf MIN_HOLE_SIZE instead, like the ls tests do
83 create_test_inputs2() 84 { 85 create_test_dir 86 87 if ! getconf MIN_HOLE_SIZE "$(pwd)"; then 88 echo "getconf MIN_HOLE_SIZE $(pwd) failed; sparse files probably" \ 89 "not supported by file system" 90 mount 91 atf_skip "Test's work directory does not support sparse files;" \ 92 "try with a different TMPDIR?" 93 fi
Jan 11 2021
@glebius: please also run rcorder libexec/rc/rc.d/* to confirm that the changes work.
Could you please update the commit message to reflect the changes being made?
Jan 9 2021
Could you please add PR: 252417 to the commit message?
The reason why I hardcoded the path was to reduce redundant $PATH lookups and to ensure that random kyua(1) binaries wouldn't get picked up from someone's environment, increasing overall determinism of the target.
Jan 6 2021
Is there a reason why tr is being changed to sed?
Dec 11 2020
Dec 8 2020
Nov 18 2020
Non-blocking thought: can this use sem_post/sem_(timed)?wait instead of spinning in busy-loops waiting for threads to start?
Nov 5 2020
Oct 30 2020
This change does more than advertised in the CR description (which doesn't describe as much as the commit message should).
Oct 28 2020
I'll look through the rest of the review later.
Oct 26 2020
I should have marked this as "Request Changes" in the last message :/...
Wait... Lua? what Lua tests are you planning to add (for the bootloader)?
Oct 20 2020
No blocking concerns.
Oct 19 2020
Oct 15 2020
This LGTM.