[Toybox] [PATCH] chmod.test: make the tests a bit more robust.

enh enh at google.com
Tue Jan 5 14:19:22 PST 2021


On Fri, Dec 18, 2020 at 3:10 AM Rob Landley <rob at landley.net> wrote:

> On 12/16/20 4:39 PM, enh wrote:
> >     While doing that, apparently host chmod -f on a directory that
> doesn't currently
> >     exist returns an error (which would make your && not continue and
> recreate it).
> >     rm -f thingy is happen if thingy doesn't already exist, but chmod -f
> isn't.
> >
> >
> > i should have said "only tested on macOS".
>
> Which I can't test on here, so may break...
>
> Sigh, this whole thing would be moot if tests/rm.test:
>
>   mkdir -p one && touch one/two && chmod 000 one
>   toyonly testing "-rf 000 dir" \
>     "rm -rf one 2>/dev/null && [ ! -e one ] && echo yes" "yes\n" "" ""
>   chmod 777 one 2>/dev/null ; rm -rf one
>
> wasn't "toyonly". MY rm got that part right.
>
> I don't remember if I made dirtree work at infinite depth yet, though. I
> came up
> with a design for it, don't think I actually implemented it yet? That's
> both a
> posix requirement _and_ a pragmatic requirement because "mv ~/bigtree ."
> can
> always create an arbitrarily deep path and then you gotta be able to get
> rid of it.
>
> While I'm using openat() for everything, I still hit filehandle exhaustion
> having too many dirfd open simultaneously. But those filehandles are only
> valid
> during the lifetime of the callback function anyway, so what I was gonna
> do was
> (at least beyond some depth threshold) keep the current and parent open,
> and
> then when it would cd .. out of the child and reopen(".") it would re-stat
> it to
> compare the cached stat info in the dirtree struct to make sure that parent
> directory's dev+inode pair hadn't changed, and just abort if it had
> because "mv
> subdir" during a traversal is pilot error and rm -rf or mv -a should
> definitely
> NOT cd .. out of a repotted deep directory and continue to merrily delete
> the
> parent directories in its new context...
>
> Anyway, I don't remember if I actually got around to implementing that, and
> haven't implemented a test for it in rm yet if I did. (I'd have to check
> the
> code and my todo lists. There's a whole "cleanup before 1.0" heap, but it
> comes
> after populating the roadmap and emptying pending.)
>
> > the reason i changed the test but not `make clean` was that i thought
> repeatedly
> > running `make test_foo` (without intervening calls to `make clean`) was
> more
> > likely to be the usual path. but maybe that's an argument for `make
> tests` and
> > `make test_foo` forcibly removing generated/testdir as if they were at
> least
> > that part of `make clean`?
>
> Possibly. I agree running tests without clean is a good thing to support,
> but
> "make clean sometimes doesn't" is a separate "we should fix that"...
>
> >     > Also, macOS seems to disallow +s in /tmp (but not otherwise), so
> >     > allow those tests to be skipped. This is why I was seeing macOS
> >     > test failures locally (because I pretty much always work on toybox
> >     > in /tmp) that we weren't seeing on the github CI runs (which
> >     > presumably don't run in /tmp).
> >     >
> >     > With this, all the tests are passing locally for me on macOS again.
> >     Needing to repeat skipnot before each line is awkward, lemme stare
> at it this
> >     evening...
> >
> > i started with a bash `if` but decided i preferred your style of
> > explicitly listing every skipped test. (though i guess `if ... else <a
> message
> > about skipping all +s tests> fi` would be fine too.)
>
> I agree that the tests should say "skipped" rather than be skipped at the
> higher
> level. I'm saying my infrastructure probably needs a "skip until further
> notice"
> flag I can set... hang on, I have one, it's "$SKIP".
>
> Try now?
>

git ToT + my locale patch (which i just resent) passes `make tests` on my
work macbook. thanks!


> Rob
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20210105/6d44ff17/attachment.htm>


More information about the Toybox mailing list