镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

Subtests Recap/Refresh #29

Closed
Closed

Description

@isaacs

Please do not comment with what you wish subtests would be, but only how they are supported in your actual real-world implementations.

There are three general forms that I see in the discussions of current implementations. (Node-tap interprets all of these, but only generates the "commented" and "buffered" styles.)

Bare Subtest

TAP version 14
1..1
    1..1
    ok 1
ok 1 - subtest name

Commented Subtest

TAP version 14
1..2

# Subtest: subtest name
    1..1
    ok 1
ok 1 - subtest name

# can be unnamed, as well:

# Subtest
    1..1
    ok 1
ok 2

The name doesn't currently have to match, but I've considered adding some strictness around that, like if a name was introduced, then any output at the parent level is considered non-TAP until we get the matching Test Point to end it.

Buffered Subtest

TAP version 14
1..1
ok 1 - subtest name {
    1..1
    ok 1
}

I have some questions about how to attach diags/directives to buffered subtests. Here's what node-tap does:

TAP version 14

ok 1 - todo subtest # TODO {
    1..1
    ok 1 - this is how node-tap generates a buffered directive subtest
}

ok 2 - todo subtest two { # TODO
    1..1
    ok 1 - node-tap supports this, but does not generate it
}

ok 3 - subtest with diags
  ---
  sub: a dub
  ...
{
    1..1
    ok 1
}

ok 4 - subtest with diags and directive # TODO
  ---
  sub: a dub
  ...
{
    1..1
    ok 1
}

1..4

We can make the spec a lot simpler if we pick a direction.

Activity

  1. mblayman commented on Mar 17, 2022

    @mblayman
    Member

    I think I punted on this whole problem with tappy because subtests are pretty uncommon in Python test suites. I'll defer to other implementation and whichever path we think will make things easiest for producers and consumers to maintain.

  2. isaacs commented on Mar 17, 2022

    @isaacs
    ContributorAuthor

    Actually specifying how bare subtests are supposed to work is proving a bit challenging. Commented and Buffered subtests are straightforward, just some of these little nits to pin down.

  3. kinow commented on Mar 17, 2022

    @kinow
    Member

    In tap4j we have the first option, bare subtest.

    Example from tests (because I didn't remember how we had actually implemented it):

    ok 1 - First test
            ok 1 - Internal subtest subtest
            1..1
        ok 1 - Internal subtest
        1..1
    ok 2 - Some subtest
    1..2
    

    I hadn't seen these other forms of subtests, interesting.

    Cheers,
    Bruno

  4. Leont commented on Mar 17, 2022

    @Leont
    ok 3 - subtest with diags
      ---
      sub: a dub
      ...
    {
        1..1
        ok 1
    }
    
    ok 4 - subtest with diags and directive # TODO
      ---
      sub: a dub
      ...
    {
        1..1
        ok 1
    }
    

    I don't think this combination is parsable in a meaningful way for any stream based parser. I don't implement it, nor do I expect to be able to implement it (other than silently ignoring it)

  5. isaacs commented on Mar 17, 2022

    @isaacs
    ContributorAuthor

    I don't think this combination is parsable in a meaningful way for any stream based parser. I don't implement it, nor do I expect to be able to implement it (other than silently ignoring it)

    Node-tap uses a stream-based parser, and supports these forms. It's not significantly more state to keep around than is already required for YAML diags attached after the main test point line. All it requires is that you hold onto the test point data until you see the first line that cannot be a YAML introduction block or a buffered subtest introduction. TAP already requires a rather involved state machine to parse it in an interruptible way (even if only because the end of a line can affect how you interpret the beginning of the line), so I'm not sure why that should be a blocker.

    But anyway, if Test::More and node-tap are the only ones to do so, then that doesn't meet the required quorum level. If all we can do is bare subtests, then that is what it is. It's not my favorite, because it's rather loosey goosey and can be hard to read with human eyes, but node-tap supports it already, so that's not a strong objection. The commented style can be suggested as a "should" nice-to-have for producers, without making it strict or suggesting warning about the absence of a comment.

    "Hard to read with human eyes" meaning:

    TAP version 14
                    1..1
                    ok 1 - quick, how many levels is this?
    

    vs

    TAP version 14
    # Subtest: first
        # Subtest: second
            # Subtest: third
                # Subtest: fourth
                    1..1
                    ok 1 - quick, how many levels is this?
    

    If we don't formalize on 4 spaces for indentation, then it is actually impossible to know how many levels that is until the end of the parse. (It could be anything between 1 and 15.)

  6. isaacs commented on Mar 18, 2022

    @isaacs
    ContributorAuthor

    So, since there's a lack of quorum on implemented support for buffered subtests, how does this sound:

    1. Subtest indentation is 4 spaces.
    2. ^# Subtest(: <name>)?$ comment may be omitted.
    3. If subtest comment is provided, Harnesses should continue subtest until matching test point (same name, or no name in either) at parent level is found, treating any un-indented lines as non-TAP, and fail the test if a correlated test point is not found.
    4. If subtest comment is not provided, Harnesses must treat the next test point at the parent level as the end of the subtest, and treat any intervening lines indented less than the parent level as non-TAP.
    5. Providers should emit a # Subtest: <name> comment if the name of the subtest is known at the outset.
    6. Harnesses should treat otherwise valid TAP (ie, version, plan, or test point) that is indented anything other than a multiple of 4 spaces as non-TAP.
    7. We'll tackle buffered subtests for TAP15, once at least one more implementation adds support matching Test::More and node-tap.

    So:

    TAP version 14
    1..2
    
      ok 1 - this is non-TAP, indented only 2 spaces
                    1..1
                    ok 1 - test point at 4th level
    
            ok 1 - this is non-TAP, it's not the right indentation level
        ok 1 - this is also non-TAP, same reason
    ok 1 - as is this
    
                ok 1 - test point closing 4th level subtest
                1..1
            ok 1 - test point closing 3rd level subtest
            1..1
        ok 1 - test point closing second level subtest
        1..1
    ok 1 - test point closing bare first level subtest
    
    # Subtest: named
        1..1
        ok
    ok 2 - this is non-TAP, wrong name
    
    # this test point closes the subtest
    ok 2 - named
    

    I'll have to update node-tap's parser a bit to add the strictness suggested here, but I think it actually makes the subtest parsing logic a bit simpler.

    My thinking is, since the subtest comment is optional, a harness can just ignore it (which seems to be what tap4j already does), and a producer can omit it, and all still be technically compliant. Test::More and node-tap both close the subtest on the first un-indented comment, even above the "expected" level, so we'll be out of spec there, but that seems like a small enough issue to address?

  7. kinow commented on Mar 18, 2022

    @kinow
    Member

    So, since there's a lack of quorum on implemented support for buffered subtests, how does this sound:

    +1 to all points, and all the _must_s and _should_s 🙆‍♂️

    I'll have to update node-tap's parser a bit to add the strictness suggested here, but I think it actually makes the subtest parsing logic a bit simpler.

    I think so too, the 4 spaces indentation will save us some trouble handling indentation. In tap4j I captured the first indentation as the current indentation level and tried to use it later, but it is messy and complicated to handle.

    My thinking is, since the subtest comment is optional, a harness can just ignore it (which seems to be what tap4j already does), and a producer can omit it, and all still be technically compliant. Test::More and node-tap both close the subtest on the first un-indented comment, even above the "expected" level, so we'll be out of spec there, but that seems like a small enough issue to address?

    +1, thanks @isaacs !

  8. added 2 commits that reference this issue on Mar 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions