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

Feature skip #59

Open
wants to merge 22 commits into
base: master
Choose a base branch
from
Open

Feature skip #59

wants to merge 22 commits into from

Conversation

hlangeveld
Copy link
Collaborator

Adds a few things:

  1. Counters for passed and skipped assertions.
  2. A skip keyword that will cause the next assert to ignore any failures and increase the skipped counter instead.
  3. The colour yellow for skipped tests.
  4. Suppression of colour output if stdin is not a terminal.

Currently based on the POSIX update.

Remove quotes from command substitution when used in an
assignment.

Two assertions were reading file VERSION where once is
sufficient.
Adds function `skip`.  If a test invokes `skip`
prior to an assertion, the assertion will be
resolved, but failing match will not result in a failure,
and instead count as a *skipped* test.

This update adds counters for passed and skipped tests.
This also changes the summary line, and color codes.

A third color is introduced for skipped tests: yellow.

The summary line will report numbers in black, with
the words 'failures', 'passed', and 'skipped' in
red, green, and yellow, respectively.

Finally, tests are included for the presence of
a terminal.  If no terminal is present, color codes
are replaced as follows:

green   OK      passed
red     NO      failed
yellow  ??      skipped
And that's 1 failure and 1 skipped assertion.
@hlangeveld
Copy link
Collaborator Author

I'm looking into adding support for an option -f <expected_failure_count> to make this produce its full output and to make it pass the full test in the case of expected failures.
Check my 'travis' branch from this feature for live updates.

@hlangeveld hlangeveld closed this Apr 19, 2015
Sometimes you want a test or assertion to Fail *visibly*.
However, we also want to use the shell `-e`, or *exit on error* option.

This extends the expected failure matcher with a temporary
suspension of that option.
Except for an example test, `shpec` itself does
not check the results of a sourced file.
shell options now enabled

-e      Exit on error. Can be suspended temporarily
for some tests.

-u      Error on undefined variables.

Checking for errors requires that we capture exit status
for failing commands with `|| _r=$?`
The `||` will prevent error exit, while the assignment
preserves the exit status for the actual assertion.
POSIX case labels can have balanced parentheses now.
Fixes a minor annoyance with editors like vi[m].
Using `skip_next_assert` is more explicit about what it does.
It implies that normal processing is restored *after* the assertion.
The function name clarifies the meaning to the reader.
The second `case` block contained only one entry.
Removed the case clutter.

Integrate failure count into final report and
`shpec` exit status/return value.
Indentation was one level to high for a command
substitution.

`-f` option had assignment and shift in one line
We renamed the keyword to `skip_next_assert` but
did not change the name of the test. Fixed now.
@hlangeveld hlangeveld reopened this Apr 19, 2015
@hlangeveld
Copy link
Collaborator Author

I would like to see some review before we even consider to pull this.
(For one thing, I forgot to bump the version. Again.)

In the shpec files, this just adds one keyword: skip_next_assert.

Skipping some assertions requires support for managing shell errors,
extra colours in output, and extra counters beyond the number of failed tests/assertions.
We now add counters for passed and skipped assertions.

In passing, this eliminated another external command by using a case statement
instead of grep. We also cater for tests without a terminal by prefixing output with
the two character codes 'NO', 'OK', and '??'.

An extra challenge was added by imposing the -e and -u options onto shpec.
It makes for more robust scripts, but potential errors are easily overlooked.

script:
- time bash bin/shpec -f 1 shpec/shpec_shpec.sh
- time dash bin/shpec -f 1 shpec/shpec_shpec.sh
- time ksh bin/shpec -f 1 shpec/shpec_shpec.sh
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Now explicitly run the test suite in all known POSIX shells.

All three shells should produce the same output.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Any specific reason we are not testing against zsh?

(if it's about the end keyword, it can be disabled, cf https://github.com/AdrieanKhisbe/shpec/blob/dev/shpec.plugin.zsh#L15)

Copy link
Collaborator

Choose a reason for hiding this comment

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

@AdrieanKhisbe @hlangeveld - see https://travis-ci.org/hlangeveld/shpec/builds/59189529
(ie. to allow failure on zsh - until you put a fix in)

Copy link
Collaborator

Choose a reason for hiding this comment

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

@locochris indeed it's the end keyword. :)

Question is where the good place to disable it?.
So far I did it in a wrapping alias. (to turn it on and off). But I thinks it might better to get this inside the script itself.

@rylnd
Copy link
Owner

rylnd commented Apr 21, 2015

@hlangeveld let's focus on getting #58 merged before pushing this any further. My main concern is that without discussing and collaborating on these features, we can easily get away from shpec's base intentions.

@hlangeveld
Copy link
Collaborator Author

On 04/21/15 21:40, Ryland Herrick wrote:

@hlangeveld https://github.com/hlangeveld let's focus on getting #58 #58 merged before
pushing this any further. My main concern is that without discussing and collaborating on these features, we can easily get away
from shpec's base intentions.

That's fine. I did some not very clean refactoring in this bit, and it got interspersed with adding the feature.
I'd rather complete the POSIX PR first, then refactor, and then we can discuss the 'skip' feature.

Cheers all,
Henk

Use advanced .travis.yml  to generate separate jobs for each shell, by @locochris
@rylnd
Copy link
Owner

rylnd commented Apr 2, 2017

@hlangeveld what is the status of this PR?

@hlangeveld
Copy link
Collaborator Author

hlangeveld commented Apr 3, 2017

Good question. Let me have a look...
.
.
.
In its current state I believe I need to refactor the whole thing again before resubmitting.
Not tonight though.

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