Skip to content
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

Make it POSIX #56

Closed
wants to merge 9 commits into from
Closed

Make it POSIX #56

wants to merge 9 commits into from

Conversation

hlangeveld
Copy link
Collaborator

Two major updates:

This removes any '...shisms' not compatible with POSIX. We're not going back to pure Bourne shell, which I consider too primitive. See the individual deltas for what changed. Two major things were:

  • Arithmetic notation, which I fixed with a hack: prefix every (( ... )) expression with : $.
  • Simple, single bracket tests: [ ... ] instead of [[ ... ]]

The second change is to throw all code into functions, this makes testing of functions simple, as we can just source the shpec script and test functions separately. It also simplifies recursive calls to
shpec. However, recursive calls do not update the assertion and failure counters, as its internals
run in a subshell, to protect the environment.

Performance: I had a simple set of 37 assertions which run in ~12 ms on my laptop.
There's little difference between bash, ksh and dash, except that bash takes an extra 8-10 ms to start itself.

I'm still looking for the shell equivalent of python's if __name__ == '__main__':
Until then I need to trigger on the value of $0.

Wrap all code in functions.  Use the name
as a trigger for actual execution.

For slightly improved speed, we actually
define `shpec` as a function.

This allows us to run shpec as a script or
as a function.  Naturally, the script just
calls the function.

To protect the environment all processing
occurs inside a subshell.
  Not all POSIX shells support `echo -e`.

  Create a function echoe() to replace
  every occurrence of `echo -e`.
Split $((( into $(( and ( to show the subexpression better.
  `(( ... ))` is not a valid POSIX arithmetic expression.

  `: $(( ... ))` is ugly, but is equivalent.

  Tested in ksh93, bash, and dash under Ubuntu 14.04
Add a warning about bash allowing the replacement
of any shell builtin.

shpec's own tests actually try to redefine `exit`,
which is a special builtin in ksh and dash and cannot
be redefined.
  Sadly, a favourite of mine. But it has to go.
  Worked fine in bash and ksh, but not in dash.

  We also split long lines here. Purely cosmetic.
As `shpec` redefines itself as a function internally,
we just call that function in the example assertions.

No need to locate the actual script. Yay!
@AdrieanKhisbe
Copy link
Collaborator

That's a greate PR! :)
Just add a look about it, sound great!
(was also thinking we should make shpec a function for more flexibility and perf)

Will try to test it out in the weekend.

Could you explain what the hack about arithmetic expression does?

(about the if main. i think the way to go in bash is with the $0)

@erichs
Copy link

erichs commented Apr 10, 2015

@hlangeveld: so awesome!

@rylnd
Copy link
Owner

rylnd commented Apr 10, 2015

😍 This looks amazing. It's quite a significant PR, so please give me some time to 1) understand it and 2) provide some feedback.

Thanks so much @hlangeveld !

@hlangeveld
Copy link
Collaborator Author

On 04/10/15 09:26, Adriean Khisbe wrote:

Could you explain what the hack about arithmetic expression does?

I think I can.

tldr; The hack just takes an assignment masquerading as an expression, but discards the string as
an argument of :, a shell builtin equivalent to true.

Detailed explanation:

(Note: In this example, I'll use the markdown convention of using ... and the four space indent
to highlight code samples.)

Let's take the very simple expression:

 : $((a=1))

It consists of two parts: : and $(( a=1 )) (arithmetic expressions are liberal about white space).

The shell builtin : is an actual command, accepts arguments, discards them, and returns status 0.
It's therefore an equivalent of true.

As : is a builtin, any side effects during its parameter substitution (which happens before it is executed)
are preserved

The arithmetic expression actually contains an assignment, and like many modern language, the value
of the assignment is equal to the value of the left hand side after assignment. Therefore,

 : $(( a=1 ))

is equivalent to:

 : 1

with a side-effect of assigning 1 to variable a. See also: (http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_04)

All changes to variables in an arithmetic expression shall be in effect after the arithmetic expansion, as in the parameter expansion "${x=value}".

Note, it is not fully equivalent, because an arithmetic statement will return
true (exit status 0) if its numeric value equals non-zero. Otherwise, it will be false (1).

 :  any parameter expansion not resulting in errors

will always return true (exit status 0).

A precedent can be found in very early *sh documentation;

 : ${variable:=value}

I believe this was already valid syntax for the Bourne shell, together with:

  • ${var:+replacement} # if $var is set, expand to "replacement".
  • ${var:-default} # expand to "default" if $var is not set (or empty)
  • ${var:?error} # send message "error" to stderr if $var not set, and exit current (sub)shell

I hope I made myself clear.

Cheers,
Henk

@AdrieanKhisbe
Copy link
Collaborator

@hlangeveld thanks for the explanation :).
Didn't know at all about the :.
(shell features are infinite ^^)

@hlangeveld
Copy link
Collaborator Author

On 04/11/15 11:39, Adriean Khisbe wrote:

@hlangeveld https://github.com/hlangeveld thanks for the explanation :).
Didn't know at all about the |:|.
(shell features are infinite ^^)

A bit of personal history: Back in '85, I had an instructor who
challenged us to write shell scripts with a minimal amount
of calls to outside code (i.e., no exec() system calls).

Somehow that stuck with me. Since reading Michael Feathers' '... Legacy Code'
I found inspiration for making shells scripts (components) test-ready.

Even if you don't actually write a test (but you should), your code will
be better if you write it with tests in mind.

Cheers,
Henk

red="\033[0;31m"
green="\033[0;32m"
norm="\033[0m"
#
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this line have a purpose?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A shell script traditionally started with just ': '. Before kernels interpreted the hasbang,
the shell would see if the first character(s) equalled ':' or ': '. In that case it would fork itself and feed itself the script.

I don't know how I ended up adding an empty comment up there.

@rylnd
Copy link
Owner

rylnd commented Apr 13, 2015

@hlangeveld all in all, this is wonderful! I've added a few comments above. Once those are resolved, we just need to update the changelog, bump the version, and merge!

@rylnd
Copy link
Owner

rylnd commented Apr 13, 2015

@hlangeveld now that shpec is a function defined as a subshell command, can you speak to whether this affects any existing issues (for better or worse), particularly #16 or #30?

@hlangeveld
Copy link
Collaborator Author

On 04/10/15 18:22, Ryland Herrick wrote:

😍 This looks amazing. It's quite a significant PR, so please give me some time to 1) understand it and 2) provide some
feedback.

Thanks for the feedback so far. Most are thinks I need to fix.
Not tonight, though.

Cheers,
Henk

@hlangeveld
Copy link
Collaborator Author

On 04/13/15 04:29, Ryland Herrick wrote:

@hlangeveld https://github.com/hlangeveld now that |shpec| is a function defined as a subshell command, can you speak to whether
this affects any existing issues (for better or worse), particularly #16 #16 or #30
#30?

With shpec redefining itself as a function this has little effect on issue #16.

The easiest way for me is to put the code executed in the tests in a subshell and test
its exit status, or to use command substitution (which implies a subshell):

 describe "example 1 ..."
   it "explain assertion"
      (
        : subshell with code
      )
      assert matcher ...
   end
 end

 describe "example 2 ..."
   it "explain assertion"
      a=$(
        : subshell with code
      )
      b=$(
        : another subshell
      )
      assert matcher referring to "$a" and/or "$b"
   end
 end

A completely different way of passing info between the framework and a project's tests would be to make
individual tests run entirely in a subshell and report results in a fixed format, to be read by the framework.
Say a single line with status, (#pass #fail "description"), followed by multiple lines of arbitrary output
from individual assertions with more detail.

@rylnd
Copy link
Owner

rylnd commented Apr 20, 2015

Closing this in favor of the newer #58

@rylnd rylnd closed this Apr 20, 2015
@rylnd rylnd removed the in progress label Apr 20, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants