Add YAML() for generating YAML blocks #7
Conversation
Docs in [1]. I've used Go's JSON serializer to avoid external
dependencies, since JSON is a subset of YAML [2]. Prove is currently
choking on the results:
$ prove test/yaml/test
test/yaml/test .. Failed 1/2 subtests
Test Summary Report
-------------------
test/yaml/test (Wstat: (none) Tests: 1 Failed: 0)
Parse errors: Unsupported YAMLish syntax: '{' at /usr/lib64/perl5/5.22.3/TAP/Parser/YAMLish/Reader.pm line 101.
Bad plan. You planned 2 tests but ran 1.
Files=1, Tests=1, 0 wallclock secs ( 0.01 usr + 0.00 sys = 0.01 CPU)
Result: FAIL
$ prove --version
TAP::Harness v3.35_01 and Perl v5.22.3
but node-tap [3] parses it fine:
$ tap test/yaml/test
test/yaml/test ........................................ 2/2
total ................................................. 2/2
2 passing (32.15ms)
ok
$ tap --version
11.0.0
[1]: https://testanything.org/tap-version-13-specification.html#yaml-blocks
[2]: http://www.yaml.org/spec/1.2/spec.html
$ curl -s http://www.yaml.org/spec/1.2/spec.html | grep -B1 JSON | head -n2
The primary objective of this revision is to bring YAML into compliance
with JSON as an official subset. YAML 1.2 is compatible with 1.1 for
[3]: http://www.node-tap.org/
|
There's an existing prove bug here. If we want to use prove before that is fixed, we could probably do so by using a YAML marshaller instead of the JSON marshaller. Personally, I prefer switching to node-tap over adding a non-stdlib Go requirement, but I could see that going either way depending on how involved with |
|
I'm hesitant to move away from prove since many Unix-like systems have it installed by default as part of their Perl installation. If an external dependency is absolutely necessary, I'd prefer a Go dependency since go get makes those easy to deal with on all platforms. Having said that, is it possible to configure JSON output into a format that prove can parse? Maybe by skipping indentation and placing it all on one line? |
I tried that, and it didn't work either. And there are no Go consumers listed here. So should I add a Go YAML dependency? |
|
Ah, I'll put it behing an optional build flag, so whoever's compiling can choose. |
So folks who are comfortable with the additional dependency can get prove-compatible output. Michael wants to stick with prove for tap-go because it is widely installed [1], but folks using node-tap or other consumers that can handle the JSON subset of YAML probably don't want the external dependency. Folks who want yaml.v2 can enable the yaml build tag [2]. Folks who are ok with JSON don't have to set any build tags. The go-yaml dependency is the only Go producer listed in [3]. There may be others, I haven't checked. The Makefile changes include new uses of wildcards [4] to pick up test/%/*.go siblings. And I'm using the stem variable $* [5] in the rule to pick up the whole test package (and not just main.go). I'm not sure how Michael wants vendoring to work. For the moment, I've softened the 'GOPATH =' to a 'GOPATH ?=' and installed the package in my local GOPATH. It's possible that we want to stick with 'GOPATH =' and drop the package under gopath/ (via a Git submodule?). [1]: mndrix#7 (comment) [2]: https://golang.org/pkg/go/build/#hdr-Build_Constraints [3]: http://yaml.org/ [4]: https://www.gnu.org/software/make/manual/html_node/Wildcard-Examples.html [5]: https://www.gnu.org/software/make/manual/html_node/Automatic-Variables.html
|
Ok, I've pushed 4656fa9 with a |
|
Good idea to use a build flag. I think it's fine to use a local GOPATH for the new dependency. I'm traveling for the next few months and won't have as much time to review and test code as usual. I'm also not actively using tap-go at the moment, so you're probably in a better position right now to judge these contributions. Your contributions so far have been good, so I've added you as a collaborator on this repository. Feel free to merge this pull request (and others) when you feel that they're ready for general consumption. I'll offer feedback as opportunity permits. |
|
On Sat, Dec 02, 2017 at 08:46:25AM +0000, Michael Hendricks wrote:
Feel free to merge this pull request (and others) when you feel that
they're ready for general consumption.
I went ahead and merged this one. The project is pretty quiet, so I'm
not expecting many more changes in the next few months.
|
Docs here. I've used Go's JSON serializer to avoid external dependencies, since JSON is a subset of YAML. Prove is currently choking on the results:
but node-tap parses it fine:
I'll work up a prove ticket once I can find out where to file it. In the mean time, would you consider switching to node-tap for this project?