Added todo directive #38

andrewfowlie · 2017-11-17T00:04:27Z

I have added the todo directive as described in the TAP specification - https://testanything.org/tap-version-13-specification.html

The syntax is similar to the skip directive:

@test "todo test that passes" {
  todo "this todo is done"
  return 0
}

@test "todo test that fails" {
  todo "this todo is outstanding"
  return 1
}

This results in

 ✓ todo test that passes (done: this todo is done)
 - todo test that fails (todo: this todo is outstanding)
   (in test file todo.tap, line 7)
     `todo "this todo is outstanding"' failed

2 tests, 0 failures, 1 todo, 1 done

and in TAP format:

1..2
ok 1 todo test that passes # done this todo is done
not ok 2 todo test that fails # todo this todo is outstanding
# (in test file todo.tap, line 7)
#   `todo "this todo is outstanding"' failed

Moving the first test case in between the two duplicate test cases caused a test failure, revealing the need to add `sort` in between `grep` and `uniq -d`.

Extracted this function to encapsulate the operation prior to future refactoring. It also fits better with the surrounding code style, whereby small functions provide documentation, even for single-line commands.

This achieves the same effect without launching a new process for the `test` command. Also, using a multi-line `if` statement and keeping the line length to under 80 columns helps with readability.

The `case` statement was a little difficult to read, and as it turns out, using `$FIXTURE_ROOT/duplicate-tests.bats` enables us to make an exact string comparison. Also added some `printf` statements to see more clearly what the results were when a test assertion fails.

The big thing I did in #8 was to eliminate as many subshells, pipelines, and subprocesses as possible. Even though we normally rely write scripts because of these features, in a framework like Bats, they add up quickly and can have a big performance impact—especially on Windows, especially when running virtualized. Conseqently, replacing the previous `grep` pipeline with a `while` loop causes the suite to run ~9% faster on macOS, and ~19% faster on Windows (running under VMWare). The following times were collected by running `time bin/bats test` on a: MacBook Pro Processor: 2.9 GHz Intel Core i5 RAM: 8 GB 1867 MHz DDR3 Bash 4.4.12(1)-release running on macOS 10.12.6 ----------------------------------------------- Before this change: 47 tests, 0 failures real 0m4.703s user 0m2.712s sys 0m1.570s After this change: 47 tests, 0 failures real 0m4.312s user 0m2.369s sys 0m1.166s Git for Windows Bash 4.4.12(1)-release running on Windows 10 under VMWare Fusion 8.5.8 ------------------------------------------------------------------ Before this change: 47 tests, 0 failures real 0m19.832s user 0m4.170s sys 0m8.893s After this change: 47 tests, 0 failures real 0m16.675s user 0m3.463s sys 0m7.297s

andrewfowlie · 2017-11-17T00:10:59Z

If you like this I can also update docs + add some tests

andrewfowlie · 2017-12-14T00:41:08Z

is it useful?

sublimino · 2017-12-14T17:52:14Z

Hi @andrewfowlie, it looks useful!

bats is currently trying to get to a place where 1.0 can be shipped, I guess this would make sense after that release? Paging @btamayo @mbland @jasonkarns

andrewfowlie · 2017-12-14T23:40:58Z

Cool. The changes are pretty small and it's useful for me because e.g., Jenkins can read TAP and knows about todos, and it'll let me write tests, then write my codes to pass the tests...

I don't know much about the development plan so have no strong opinions about whether it should be in 1.0 or later.

Addresses sstephenson/bats#240. When outputting a 'pretty' formatted output stream, the 'skip' message was being processed with a character class of [^)]. I couldn't find a reason for this to be the case, so I've swapped it out to check instead of printable characters, spaces included ([:print:]). Please advise if there is a problem with this approach. Tested using Bash 4.3.11(1)-release.

Two things here: - judging by the "Added ..." lines of the changelog, this justifies pumping to 0.5.0 instead of a patch release, ie 0.4.1 - I changed the order of mentioning changes, I mention first the new stuff ("Added..."), then the improved stuff and then the bug fixes.

mbland · 2018-06-01T16:35:28Z

libexec/bats-exec-test

@@ -80,6 +80,11 @@ skip() {
  exit 0
 }

+BATS_TEST_TODOED=''
+todo() {
+  BATS_TEST_TODOED=${1:-1}


The ${1:-1} needs to be double-quoted.

mbland

The comments I've made here carry throughout the rest of the changes I didn't comment on. I realize some of the changes go along with the existing code style (i.e. using [ instead of [[), but go ahead and use the new style.

This'll probably be a 1.0 feature. You'll need to rebase/merge from master as needed. Whether you want to do that now or until it's about ready to merge is up to you.

We'll also need tests for this, of course.

mbland · 2018-06-01T16:35:53Z

libexec/bats-exec-test

@@ -270,14 +277,23 @@ bats_exit_trap() {
    fi
  fi

+  if [ -n "$BATS_TEST_TODOED" ]; then


Use [[ and ]] for all new code, please.

mbland · 2018-06-01T16:36:06Z

libexec/bats-exec-test

+  if [ -n "$BATS_TEST_TODOED" ]; then
+    todoed=" # todo"
+    todone=" # done"
+    if [ "1" != "$BATS_TEST_TODOED" ]; then


[[ and ]].

mbland · 2018-06-01T16:37:19Z

libexec/bats-format-tap-stream

@@ -51,6 +53,22 @@ skip() {
  advance
 }

+todo() {
+  local reason="$1"
+  [ -z "$reason" ] || reason=": $reason"


Use [[ and ]], and expand this into an if statement to make it easier to scan.

andrewfowlie · 2018-06-02T10:29:15Z

Thanks for the comments - will make the changes soonish

…patch-3

It appears as though this test didn't actually test what it was supposed to in 25505bd ("Skip pretty formatting if the first line isn't a TAP plan"). It was supposed to test that bats-format-tap-stream would not try to format non-TAP data -- but instead it just showed that the preamble of test files was executed before printing the TAP header (something that is not going to be the case with the suite refactor). Fixes: 25505bd ("Skip pretty formatting if the first line isn't a TAP plan") Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>

This allows us to remove most of the complicated handling between bats-exec-test and bats-exec-suite (which had a complicated interplay due to the fact they both supported multi-test execution). Instead, multi-test execution is entirely left to bats-exec-suite and bats-exec-test only handles a single test. This entirely removes the need for the magic output handling in bats-exec-test -- and opens the door to suite-wide parallelism in one fell swoop. The only downside is that bats-preprocess is run multiple times on the same source file, but a follow-up patch will improve the caching of pre-processed files such that they will only be processed once. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>

Most well-behaved bats tests (where they aren't writing to shared data between test executions) are able to be executed in parallel, but to date bats has always executed them in serial. This is incredibly wasteful for trivially parallelisable tests, and can result in fairly inflated test times for no good reason. Parallel exeuction is done suite-wide, so it makes no difference how the tests are structured. Unfortunately, because some suites might be implicitly serial (like bats' own test suite) we cannot make the default mode execute in parallel. In addition, there is currently no way of tagging that a *specific* test has to be run in serial (so that the rest can be run in parallel) -- but that can be done in future work. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>

Testing that parallel execution actually works is a bit difficult, so the most trivial way of doing it is to just have a bunch of sleep tests and make sure that they run faster than would be possible if they were run in serial. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>

@cpswan

Signed-off-by: Chris Swan <@cpswan>

Added instructions for installing on Git Bash

Tweaked some wording after #143

bats: support parallel execution of tests

Aazhar · 2019-06-12T09:42:13Z

Hello,
I would like to know if this feature will merged anytime soon ?
Thanks

sublimino · 2019-06-13T09:38:06Z

@andrewfowlie (due to the passing of time, sorry!) this branch has some conflicts, could you rebase master onto it and ensure the tests still pass?

In general LGTM, although I'd like a second opinion and to test this in anger for a while. If you'd like to test the latest (e.g. rebased) version of this @Aazhar please do so and let us know here : ]

Aazhar · 2019-06-16T12:35:36Z

ok so after reviewing this PR, I'm questioning the meaning of 'done' todos in the initial post.
Actually imho, all todos should be counted as todos, regardless of their supposed result.

andrewfowlie · 2019-06-16T13:20:44Z

I will try to rebase next week. Done means that the todo is complete because the test passes. I think it’s useful to indicate that (although could change the wording)

…

On 16 Jun 2019, at 8:35 pm, Aazhar ***@***.***> wrote: ok so after reviewing this PR, I'm questioning the meaning of 'done' todos in the initial post. Actually imho, all todos should be counted as todos, regardless of their supposed result. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

this was complicated as my modified files were since removed from repo

andrewfowlie · 2019-06-17T03:26:16Z

arggh, something has gone wrong with my PR/rebase against master. There was some small complication because some of my modified files had been removed from the upstream repo, so I did some things manually. But I thought it worked (master...andrewfowlie:master), but this PR seems to include all kinds of commits and changes.

Maybe it's this problem - https://stackoverflow.com/questions/18323665/pull-request-on-github-showing-commits-rebased-from-master

jasonkarns · 2019-07-14T14:50:17Z

I'm assuming part of the problem here is that the changes were made on the master branch instead of on a feature branch.

jasonkarns · 2019-07-14T14:59:37Z

As to "done" todos...

@Aazhar the TAP spec itself indicates that tests marked as TODO that pass should be counted as "bonus" tests.

Personally I disagree with the TAP spec here. I believe rspec's behavior is better here. A test that's marked as TODO should not pass. If it does, the test is either not a valid test (it's testing the wrong thing) and needs to be updated; or the test is valid and the functionality is complete, in which case, the test needs to be updated (remove the TODO marker). Ergo, in both scenarios, the test needs "fixed". Rspec reports pending tests that pass as failures, and I believe that is the proper behavior. However, since the TAP spec is explicit, and at least TAP reports them separate from regular passing tests, I'll concede that spec-compliant behavior is acceptable.

However, as a test suite that contains DONE TODOs requires changes, I think we should follow rspec's lead here and return a non-zero exit for the suite in such a case.

andrewfowlie · 2019-07-15T16:47:15Z

That makes sense. I am very busy travelling right now. I’m happy for anyone to finish this PR/if anyone knows how to fix the aforementioned GitHub problem with the PR please let me know

dotmpe and others added 7 commits October 14, 2017 11:45

Initial code and test for duplicate test-name warning

c67e2c7

dup-tests: Emit warning when tests out-of-order

7426182

Moving the first test case in between the two duplicate test cases caused a test failure, revealing the need to add `sort` in between `grep` and `uniq -d`.

dup-tests: bats_detect_duplicate_test_case_names

4d7287a

Extracted this function to encapsulate the operation prior to future refactoring. It also fits better with the surrounding code style, whereby small functions provide documentation, even for single-line commands.

dup-tests: Use builtin instead of test command

3160ede

This achieves the same effect without launching a new process for the `test` command. Also, using a multi-line `if` statement and keeping the line length to under 80 columns helps with readability.

Added todo directive

e5bb6e9

Add bad command line option errors

3754d15

cdevinesr and others added 4 commits December 20, 2017 15:58

Initial code and test for duplicate test-name warning

fc41eb7

Perf

ea5787f

mbland reviewed Jun 1, 2018

View reviewed changes

mbland suggested changes Jun 1, 2018

View reviewed changes

sublimino and others added 5 commits June 3, 2018 23:24

docs: update release notes

3315064

match bats-core style

db5ebe2

changed names of new variables to be easier to read

8443e7e

switch to double brackets

7a4e580

merged master and fixed minor conflict

256ba8e

andrewfowlie requested a review from a team as a code owner June 4, 2018 01:17

andrewfowlie and others added 5 commits June 4, 2018 11:22

first commit of tests for todo/done

86e07bb

Replaces commit with github issue number

c626b7a

Brings back lost word

266557f

Merge branch 'master' of https://github.com/bats-core/bats-core into …

c314a3b

…patch-3

Merge branch 'dup-warning-#9' into features/duplicate-test-warning

8c3e6e1

cyphar and others added 10 commits February 11, 2019 02:01

Added instructions for installing on Git Bash

c2efe31

Signed-off-by: Chris Swan <@cpswan>

docs: added wording for $PATH in Git Bash

e4312a9

Merge pull request #180 from cpswan/cpswan-git-bash

8b0fe23

Added instructions for installing on Git Bash

Update ToC for README.md

6bb9a5d

Tweaks to readme (#185)

0cd82db

Tweaked some wording after #143

Merge pull request #172 from cyphar/bats-parallel

8789f91

bats: support parallel execution of tests

andrewfowlie and others added 9 commits June 17, 2019 10:43

Added todo directive

681e518

match bats-core style

848ddd5

changed names of new variables to be easier to read

d5bb5d3

switch to double brackets

d68fb54

first commit of tests for todo/done

80126bb

manually merge todo

e865ab5

this was complicated as my modified files were since removed from repo

match newer bats style/syntax

bb7e7d7

fixed problems with new style/syntax

791f2bf

fix merge

035b6bc

sublimino mentioned this pull request Jun 19, 2019

Noskip option flag sstephenson/bats#264

Open

andrewfowlie closed this Apr 13, 2020

andrewfowlie mentioned this pull request Apr 13, 2020

add todo and done features #275

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added todo directive #38

Added todo directive #38

andrewfowlie commented Nov 17, 2017

andrewfowlie commented Nov 17, 2017

andrewfowlie commented Dec 14, 2017

sublimino commented Dec 14, 2017

andrewfowlie commented Dec 14, 2017

mbland Jun 1, 2018

mbland left a comment

mbland Jun 1, 2018

mbland Jun 1, 2018

mbland Jun 1, 2018

andrewfowlie commented Jun 2, 2018

Aazhar commented Jun 12, 2019

sublimino commented Jun 13, 2019

Aazhar commented Jun 16, 2019

andrewfowlie commented Jun 16, 2019 via email

andrewfowlie commented Jun 17, 2019

jasonkarns commented Jul 14, 2019

jasonkarns commented Jul 14, 2019

andrewfowlie commented Jul 15, 2019

Added todo directive #38

Added todo directive #38

Conversation

andrewfowlie commented Nov 17, 2017

andrewfowlie commented Nov 17, 2017

andrewfowlie commented Dec 14, 2017

sublimino commented Dec 14, 2017

andrewfowlie commented Dec 14, 2017

mbland Jun 1, 2018

Choose a reason for hiding this comment

mbland left a comment

Choose a reason for hiding this comment

mbland Jun 1, 2018

Choose a reason for hiding this comment

mbland Jun 1, 2018

Choose a reason for hiding this comment

mbland Jun 1, 2018

Choose a reason for hiding this comment

andrewfowlie commented Jun 2, 2018

Aazhar commented Jun 12, 2019

sublimino commented Jun 13, 2019

Aazhar commented Jun 16, 2019

andrewfowlie commented Jun 16, 2019 via email

andrewfowlie commented Jun 17, 2019

jasonkarns commented Jul 14, 2019

jasonkarns commented Jul 14, 2019

andrewfowlie commented Jul 15, 2019