Skip to content

Commit

Permalink
Handle test.test being run as root.
Browse files Browse the repository at this point in the history 
Root always has read and write access.

Tested with `sudo make test_test`
  • Loading branch information
@jfgoog @landley
jfgoog authored and landley committed May 24, 2022
1 parent f628d68 commit 9febe91
Showing 1 changed file with 5 additions and 2 deletions.
7 changes: 5 additions & 2 deletions tests/test.test
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,13 @@ touch walrus
MASK=111
for i in x w r k g u; do
[ $i == k ] && MASK=1000
XX=no
[ $UID -eq 0 ] && [ $i == r -o $i == w ] && XX=yes # Root always has access
# test everything off produces "off"
chmod 000 walrus
testcmd "-$i 0" "-$i walrus || echo yes" "yes\n" "" ""
testcmd "-$i 0" "-$i walrus && echo yes || echo no" "$XX\n" "" ""

This comment has been minimized.

Sign in to view

Copy link
@jfgoog

jfgoog May 24, 2022

Author Contributor

Grr...on-device tests are still failing.

FAIL: test -w 0
echo -ne '' | "/system/bin/test" -w walrus && echo yes || echo no
--- expected	2022-05-24 20:47:27.303993485 +0000
+++ actual	2022-05-24 20:47:27.327993484 +0000
@@ -1 +1 @@
-no
+yes

This comment has been minimized.

Sign in to view

Copy link
@jfgoog

jfgoog May 24, 2022

Author Contributor

I wonder if $UID isn't set, and we need to actually do $(id -u)?

Elliott?

This comment has been minimized.

Sign in to view

Copy link
@enh-google

enh-google May 24, 2022

Collaborator

yeah, $UID is a bash-ism, and that's why the other similar cases in here all call id(1).

This comment has been minimized.

Sign in to view

Copy link
@jfgoog

jfgoog May 24, 2022

Author Contributor

Tested with $(id -u) and that seems to have worked. I'll send a new change.

This comment has been minimized.

Sign in to view

Copy link
@landley

landley May 24, 2022

Owner

Sorry, I keep testing against bash and toysh. I'm currently trying to get toysh to run the test suite, but I hit a really weird bug where $PWD and actual cwd (/proc/$$/cwd) get out of sync in a chroot and I'm trying to track it down while I've got an isolated (if elaborate) reproduction sequence (which involves interrupting a command with a ctrl-c at the right moment, followed by multiple relative cd commands). Plus I've been sick and my roommate who had it first was down with this flu all last week...

This comment has been minimized.

Sign in to view

Copy link
@jfgoog

jfgoog May 25, 2022

Author Contributor

No worries. It makes perfect sense that you would test with bash and toysh by default. Would it be worth the effort to set up some CI (docker images?) to test with some other shells?

Hope you feel better soon. There is no particular urgency on my end, and even if there were, I hope you prioritize your health over merging my patches.

This comment has been minimized.

Sign in to view

Copy link
@landley

landley May 25, 2022

Owner

I could set it up here myself, but what I want to do is get to the point where the toybox test suite can dogfood the toybox shell. (TEST_HOST would still be /bin/bash, which is what scripts/test.sh says at the top. IETF bakeoff rules: two interoperable implementations should exist.)

The only "other shell" that matters is the one you guys use for your tests (mksh?). The toybox test suite specifies bash and does not try to be portable to "generic shell implementation", but the real world use case available right now is that bash isn't deployable for license reasons and toysh isn't ready yet, so it has to also run under mksh for now. Elliott's been trimming bashisms from the tests for years, and I need to finish toysh. (I am not wording properly right now, I hope the above was reasonably coherent.)

This comment has been minimized.

Sign in to view

Copy link
@enh-google

enh-google May 26, 2022

Collaborator

(yeah, my view here is still "worrying about mksh is not a good use of your time" --- it's neither a big nor a frequent problem, and it's easier for us to test in context, so i think it makes sense for us to worry about that. and an intermediate phase where we still use mksh as sh but toybox as bash solves this anyway as soon as toybox's shell is able to run its own tests.)

This comment has been minimized.

Sign in to view

Copy link
@jfgoog

jfgoog May 26, 2022

Author Contributor

Yeah, especially since I belated realized I can test toybox changes locally in our repo before sending them upstream.

This comment has been minimized.

Sign in to view

Copy link
@enh-google

enh-google May 26, 2022

Collaborator

(google-only note: yeah, and bear in mind you don't even need a legit build server prebuilt to test with; you can upload any random binary and run presubmit with that --- outsiders can't [for obvious reasons], but as a googler, there's nothing to stop you and it's completely fine. you just can't check in one of those random binaries [for obvious reasons]!)

This comment has been minimized.

Sign in to view

Copy link
@jfgoog

jfgoog May 26, 2022

Author Contributor

Yeah, in case it wasn't clear, I was referring to presubmit tests of changes which I always, by habit, mark DO NOT SUBMIT.
chmod $((7777-$MASK)) walrus
testcmd "-$i inverted" "-$i walrus || echo yes" "yes\n" "" ""
testcmd "-$i inverted" "-$i walrus && echo yes || echo no" "$XX\n" "" ""
MASK=$(($MASK<<1))
done
unset MASK
Expand All @@ -63,6 +65,7 @@ for i in uu+s gg+s k+t; do
done
# test each ugo+rwx bit position individually
XX=no
[ $UID -eq 0 ] && XX=yes # Root always has access
for i in 1 10 100; do for j in x w r; do
chmod $i walrus

Expand Down

0 comments on commit 9febe91

Please sign in to comment.