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

added new matches to have_a_field #8

Merged
merged 3 commits into from
Jul 27, 2017
Merged

Conversation

marcgreenstock
Copy link
Contributor

This addresses #3 by adding a with_property matcher to have_a_field

Additionally I have added with_hash_key and with_metadata.

A matcher could be added to the :as property on Argument too but I've left it out of this PR for now to keep it as small as possible.

I have a few other ideas for additional matchers on have_a_field.

It would be nice to have a with_description. The argument could be optional to test if a field has a description, or with an argument that the description matches.

Possibilities are endless, I'll add my ideas to the issues.

new matches are `with_property`, `with_hash_key` and `with_metadata`
@marcgreenstock
Copy link
Contributor Author

Codeclimate is complaining, perhaps it would be good idea to accept a standard like rubocop that runs locally in a rake task after the specs have passed.

@khamusa
Copy link
Owner

khamusa commented Jul 26, 2017

@marcgreenstock there is already a .rubocop.yml. I'm not sure what you meant, can you explain?

Anyway, most issues from codeclimate are of line length. The most critical is the length of the HaveHaveAField class. Do you think you can extracting the with_* behavior ?

@khamusa
Copy link
Owner

khamusa commented Jul 26, 2017

Also, considering the potential developments of issue #9, I guess it's mandatory we refactor that class

@marcgreenstock
Copy link
Contributor Author

@khamusa if the rakefile was

require "bundler/gem_tasks"
require "rspec/core/rake_task"
require "rubocop/rake_task"

RSpec::Core::RakeTask.new(:spec)
RuboCop::RakeTask.new

task :default => [:spec, :rubocop]

running $ rake will fail if there are any offences (which there are 86 in my branch). Contributors can rely on local checks instead of codeclimate failing the PR.

@khamusa
Copy link
Owner

khamusa commented Jul 26, 2017

@marcgreenstock got it! Good idea! Feel free to change it now, or if you prefer i'll change it myself later tonight!

@marcgreenstock
Copy link
Contributor Author

I did a bit of refactoring with #8, I wanted the description to respect the order of the methods in the chain. So if someone were to write:

it { is_expected.to have_a_field(:id).with_metadata(foo: true).with_property(:post_id).of_type('ID!') }

The description would be something like:

define field id, with metadata {:foo => true}, reading from the post_id property, of type ID!

Fail messages will also occur in the same order, with only the explaining of first failing match in the chain.

The class is a little big now, but I can't see a reasonable way of reducing it without a bit of complex metaprogramming. But I'll see what I can do. Leave this PR open for now until I've addressed some of the offences.

@marcgreenstock
Copy link
Contributor Author

I've managed to satisfy codeclimate, but there are a number of offences still in rubocop. So I won't change the rakefile until those are cleaned up.

@khamusa khamusa merged commit a473fbe into khamusa:master Jul 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants