"Fix" acceptance tests

[plexus]

It seems there were a couple of problems with the acceptance test, the biggest being that the let() blocks defined in shared example groups were interfering with each other. To prevent this I've wrapped them all in context {} blocks. Now the tests break though, because there were some issues going unnotticed.

Also added a new "list of quotes" fixture to test/document the behavior of rendering a top level collection, and renamed the fixtures for collection json to use the right naming convention.

We already exposed several of the pipeline steps as isolated methods, this adds "format" to that list, so you can turn a Yaks::Resource into a formatted data structure targeting a specific output format, without serializing it to string.

The files in yaks/spec/json/ are serialized representations of the data loaded from yaks/spec/yaml/. The JSON-API version had drifted from the defined data, and because our acceptance tests were broken we did not catch it.

Occasionally I drop a binding.pry in a test to inspect some values, in that case I don't want to have it time out immediately. In stead of commenting out the relevant section each time (and accidentally committing that :P) I can now do NO_TIMEOUT=1 bundle exec rspec

[plexus]

Anyone like to do a code review on this? I'd like to merge so we can in a next step improve CJ's handling of self links (issue #87), and bring the JSON-API reader in line with its writer.

And really I just want to write the Siren formatter. Guess why it's called Yaks :stuck_out_tongue:

/ping @groyoh @janko-m @chastell

[groyoh]

I can review this later today :wink:

[janko]

This looks really good. It's great that you started each shared examples with describe block, so that you don't have to have those empty context { ... }. It's also great that you added NO_TIMEOUT, I also missed being able to binding.pry easily. How come you set timeouts in the first place?

Just one very slight opinion, I think we can ingore some Rubocop complaints, for example eql(foo: "bar") instead of eql({foo: "bar"}). In the context of JSON testing I prefer the second version, because we semantically are testing hashes and not options. The same with some other Rubocop complaints you may not agree on, I think we don't have strictly follow it, Rubocop doesn't always know best :wink:

Is Siren good?

[groyoh]

One or two remarks, but it looks good to me! :+1:

@janko-m we could add this to the rubocop config:

Style/BracesAroundHashParameters:
  Exclude:
    - yaks/spec/*

so that it is only disabled for specs.

[plexus]

How come you set timeouts in the first place?

This is for Mutant. It is possible that a mutation results in an infinite loop, causing Mutant to hang. Actually we could probably enable the timeouts only when running mutant.

Rubocop doesn't always know best

Totally. This is the first time I'm enforcing it on a full project, so I'm still in the phase of getting a feel for it. In general I don't always like tools telling me what to do, but I want to stick with it for a bit to find out what parts about it I like and don't like. In general I do really appreciate having consistent styling across a project.

In these tests I agree an explicit hash would be more clear. I'll add the exclusion as suggested.

Is Siren good?

Yes :) From what I've seen it's the only format with links and forms that is really gaining traction. There are already a bunch of companies using it in production, and there's an active community around it. It's also a very good match for Yaks because its data model is quite close to Yaks::Resource. So I really want to get this in so I can reach out to that community.

[groyoh]

@plexus I'm for enabling timeouts only with mutant :+1:

[chastell]

This is the first time I'm enforcing [RuboCop] on a full project, so I'm still in the phase of getting a feel for it. In general I don't always like tools telling me what to do, but I want to stick with it for a bit to find out what parts about it I like and don't like. In general I do really appreciate having consistent styling across a project.

Oh definitely – I really like consistency, much more than agreeing with RuboCop’s defaults. :)

In these tests I agree an explicit hash would be more clear. I'll add the exclusion as suggested.

You can tell RuboCop to enforce braces in these cases.

[plexus]

Yeah but 9 times out of 10 I don't want the braces. It's only when explicitly testing for data that I do want them.

[plexus]

I guess that's it. Will wait for the build to finish and then merge.

[plexus]

I have to say as much as I enjoy pushing to master, it's darn nice to be able to get some feedback :smile: