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

support skipped tests #57

Open
erichs opened this issue Apr 15, 2015 · 23 comments
Open

support skipped tests #57

erichs opened this issue Apr 15, 2015 · 23 comments

Comments

@erichs
Copy link

erichs commented Apr 15, 2015

Thanks @rylnd for this awesome project! Any thoughts on how to implement skipped tests?

@AdrieanKhisbe
Copy link
Collaborator

I think I might have one:

 pending it 'should do something that is not specified yet' <<end
       assert blablabla
       assert etc
  end

Using the end here doc make that we just have to modify the one line to make the test pending. :)
Then the pending function could just print the test message, and ignore the stdin.

@erichs
Copy link
Author

erichs commented Apr 16, 2015

@AdrieanKhisbe: Nice! I definitely like the approach of adding the pending() function, but I'd be concerned that the heredoc would end up being brittle, requiring either leading tab characters, or not working correctly for various indentation styles.

What if pending() set a flag, assert() printed the test message but did nothing else when the flag was set, and end() cleared the flag?

@rylnd
Copy link
Owner

rylnd commented Apr 16, 2015

@erichs the flag seems like the simplest solution given the current behavior of shpec.

However, it's important to note that any other normal expressions within the pending test would still be evaluate, so "side effects" would have to be taken into account, which seems odd for a pending test. In other words, there's currently no way not to run a test (we can only choose whether it 'counts', not whether it's run).

@locochris
Copy link
Collaborator

@rylnd
Copy link
Owner

rylnd commented Apr 16, 2015

@locochris yeah, I was thinking the same thing. xit will be more familiar to folks coming from RSpec.

@AdrieanKhisbe
Copy link
Collaborator

Even if I touch a lot of rspec this week, didn't know about xit.

What I would say, is that it cost us nothing to do both! :) Besides event if xit is faster to write, pending is faster to read and spot.

@AdrieanKhisbe
Copy link
Collaborator

About remark of the heredoc Noting would prevent the user to use something else than end.
(true I forgot about the act that to close the hererdoc, end can't have trailling spaces)

describe 'My wondrfull super test
    pending it 'should do something that is not specified yet' <<dis
        assert blablabla
        assert etc
    end
dis
end

@AdrieanKhisbe AdrieanKhisbe added this to the Extra language features 0.2 milestone Apr 17, 2015
@erichs
Copy link
Author

erichs commented Apr 17, 2015

The above really isn't too bad for manually marking tests pending, @AdrieanKhisbe. I kind of like the simplicity. What I'd love though, is the ability to dynamically mark tests pending at run time, and have all statements ignored until the next end is encountered.

That might technically be possible using a DEBUG trap handler, but it's a horrible hack...

@AdrieanKhisbe
Copy link
Collaborator

what do you mean dynamically mark them at pending time?
Matching some criteria, tag attached?

An alternative would be that the test are function definition.

 it_"is suppose to be a some test"() {
        some testing logic
 }

no code is run before function is invoked
and then we retrieve the tests with function matching.

But I'm not sure that is really a direction we should go into.

@erichs
Copy link
Author

erichs commented Apr 17, 2015

@AdrieanKhisbe, the other day, I had to do a runtime check, and conditionally execute various tests, like:

if connected_to_vpn; then
  describe "when vpn-connected, hosts are reachable"
    it "pings myhost1"
      assert pingable myhost1 192.168.1.1
    end

    ...

  end
else
  echo "not attached to vpn, skipping"
fi

What I'd love to be able to say, is:

  describe "when vpn-connected, hosts are reachable"
    it "pings myhost1"
      connected_to_vpn || pending
      assert pingable myhost1 192.168.1.1
    end

    ...

  end

Having to surround the tests with if/else conditionals broke the feel of the DSL, and started me thinking.

@AdrieanKhisbe
Copy link
Collaborator

Okey. Now I perfectly see the use case :)

@AdrieanKhisbe
Copy link
Collaborator

I think the way to go is to "impose" the use of run async command (funciton so reconfigurable, behavior depending a global variable), I started to introduce in the prototyping branch of new language feature I started some week ago.

(problem is that would put a constraint on interpolation)

describe "when vpn-connected, hosts are reachable"
    it "pings myhost1"
      run connected_to_vpn || pending
      assert pingable myhost1 192.168.1.1
    end
# ....

@rylnd
Copy link
Owner

rylnd commented Apr 17, 2015

@erichs @AdrieanKhisbe I think that an xit (that sets a flag and then calls it) and a pending function (that simply sets the flag), along with end unsetting the flag should get us most of the way there.

@AdrieanKhisbe I'm not sure what you're getting at with the run stuff. . . can we move that conversation to #53?

@AdrieanKhisbe
Copy link
Collaborator

I do agree.

gonna start to work on this after I rebase on @hlangeveld pull request

@hlangeveld
Copy link
Collaborator

On 04/18/15 13:23, Adriean Khisbe wrote:

I do agree.

gonna start to work on this after I rebase on @hlangeveld https://github.com/hlangeveld pull request

I will try to clean up the PR and address most issues this weekend.

@hlangeveld
Copy link
Collaborator

On 04/18/15 13:23, Adriean Khisbe wrote:

I do agree.

gonna start to work on this after I rebase on @hlangeveld https://github.com/hlangeveld pull request


Reply to this email directly or view it on GitHub #57 (comment).

Oops, I think I've got a simple mechanism working for this.

I made the changes @rylnd requested. In the morning I'll check the looks of the code,
push them, and issue another PR.

@AdrieanKhisbe
Copy link
Collaborator

No hurry, I don't plan to start before monday at best. :)

@hlangeveld
Copy link
Collaborator

Well, I think I've got something working: A keyword skip_next_assert.
It's simple in its objectives. We don't skip full tests (yet), just a single assertion.

See the travis-ci output for what it looks like.

@AdrieanKhisbe
Copy link
Collaborator

That could be useful indeed :)

One suggestion, (on seems to be easily implementable according https://github.com/hlangeveld/shpec/blob/feature_skip/bin/shpec#L13), is to take in argument the number of asserts to be skipped.

 # blablabla
 skip_next_assert 42

 # blablabla

@stephenchu
Copy link

Any latest incarnation on this skip-test feature?

@rylnd
Copy link
Owner

rylnd commented Nov 19, 2018

@stephenchu thanks for your interest. I'm personally not sure where this feature is at; it may be easier to write this from scratch than to try and resurrect an old implementation.

Could you please explain the use case that you're trying to solve? I'd like to ensure that if this gets implemented, it's done so in a way that is useful to those that want it.

@stephenchu
Copy link

@rylnd Sure. I got onto this issue page only because I wanted to do the equivalent of xit in RSpec:

the output should contain "3 examples, 0 failures, 3 pending"

The desired behavior is to ignore the test body entirely, without resorting to comment the test out.

@rylnd
Copy link
Owner

rylnd commented Nov 30, 2018

@stephenchu I will try to get to this by the end of the year, but I'll be honest: it's not a priority for me. It seems like a fairly straightforward feature if you wanted to take a crack at it; PRs are always welcome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants