Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I've written tests that use Bash for testing a custom shell. I think the tests work but I'm sure you know better because this is my first test I write this way.

#!/bin/bash
echo "-- Testing our implementation of OpenShell --"
echo ""
echo "- If you have any problem in passing a test read the corresponding"
echo "- source file to understand what the test is checking"
echo ""
echo -n "********************* PRESS ENTER TO RUN TESTS  ... "
read answ
echo ls -al openshell.*|./shell
echo -n "********************* TEST ALGORITHMS ... "
read answ
./shell < <(echo "ls -al|grep open|awk '{print \$9}'")
echo -n "********************* TEST DONE. YOU SHOULD SEE FILENAMES ABOVE ... "

Now I run the test and see what I expect, my command interpreter executes the pipeline from the Bash script.

$ ./RUN_TESTS 
-- Testing our implementation of OpenShell --

- If you have any problem in passing a test read the corresponding
- source file to understand what the test is checking

********************* PRESS ENTER TO RUN TESTS  ... 
Current working dir: /home/dac/ClionProjects/shell2/openshell
'PATH' is set to /usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin.
stdin is a file or a pipe

p[0][0] ls
p[0][1] -al
p[0][2] openshell.h

-rw-rw-r-- 1 dac dac 1439 maj  1 16:45 openshell.h
********************* TEST ALGORITHMS ... 
Current working dir: /home/dac/ClionProjects/shell2/openshell
'PATH' is set to /usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin.
stdin is a file or a pipe
 {ls} {-al} {|} {grep} {open} {|} {awk} {{print $9}} {|}
p[0][0] ls
p[0][1] -al
p[1][0] grep
p[1][1] open
p[2][0] awk
p[2][1] {print $9}

openshell-0.16430.tar.gz
openshell.h
********************* TEST DONE 
share|improve this question
up vote 2 down vote accepted
+50

Instead of multiple echo statements, consider using a here document:

cat <<EOF
-- Testing our implementation of OpenShell --

- If you have any problem in passing a test read the corresponding
- source file to understand what the test is checking
EOF

To echo an empty line, you can write echo without parameters instead of echo.

The flags of echo are not portable. Instead of echo -n, you can use printf.

If you don't need the user input, you don't need a parameter for read.

Parsing the output of ls -l is considered a bad practice.

Instead of this:

./shell < <(echo "...")

You can use a here string:

./shell <<< "..."

Some vertical space (blank lines) can improve the readability of your code.

Putting it all together, your script would be better this way:

#!/bin/bash
cat << EOF
-- Testing our implementation of OpenShell --

- If you have any problem in passing a test read the corresponding
- source file to understand what the test is checking

EOF
printf "********************* PRESS ENTER TO RUN TESTS  ... "
read

echo ls -al openshell.* | ./shell
printf "********************* TEST ALGORITHMS ... "
read

./shell <<< "ls -al|grep open|awk '{print \$9}'"
printf "********************* TEST DONE. YOU SHOULD SEE FILENAMES ABOVE ... "

Update

As per your follow-up question, and as @Mat pointed out, neither <(cmd) nor here-strings will work in dash. To make the last statement work, you could use a here document:

./shell << EOF
ls -al|grep open|awk '{print \$9}'
EOF
share|improve this answer
    
Will it also work with dash and ksh since I'm targeting OpenBSD and Ubuntu? ? Since I'm writing a shell myself that we call osh, can I use my shell version 0.97 to test my shell version 1.0 ? – Dac Saunders May 2 at 4:33
1  
Yes, or should work in dash and ksh too – janos May 2 at 5:04
    
Why did you say this: "Parsing the output of ls -l is considered a bad practice." ? Why is it so? I changed the test and asked about it again here. Thanks for the help. – Dac Saunders May 2 at 5:06
1  
@Programmer400: mywiki.wooledge.org/ParsingLs; I don't think here strings are portable (mywiki.wooledge.org/Bashism says not), and process substitution isn't available on all OSes. – Mat May 2 at 5:08
1  
Thanks @Mat, I actually misunderstood the question. Added a paragraph about this to my answer – janos May 2 at 5:23

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.