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

[Fix #80] Ability to preserve traits #83

Merged
merged 4 commits into from Jun 28, 2018

Conversation

Projects
None yet
2 participants
@Vasfed
Contributor

Vasfed commented Jun 26, 2018

Fixes #80.

This adds option to TestProf::FactoryDefault to revert to default behaviour when factories have traits for association.

@Vasfed Vasfed force-pushed the Vasfed:feature/preserve_traits branch from 09ad22f to d45937e Jun 26, 2018

end
def set_factory_default(name, obj)
FactoryDefault.register(name, obj)
def set_factory_default(name, obj, preserve_traits = nil)

This comment has been minimized.

@palkan

palkan Jun 28, 2018

Owner

Let's make it a keyword argument and pass it as .register(name, obj, options). We'll probably have more options in the future.

def register(name, obj, preserve_traits = nil)
unless FactoryDefault.preserve_traits
@store_preserve_traits[name] ||= true if preserve_traits
end
store[name] = obj

This comment has been minimized.

@palkan

palkan Jun 28, 2018

Owner

What about storing the options along with the object itself, i.e.:

store[name] = { record: obj, **options}

This comment has been minimized.

@Vasfed

Vasfed Jun 28, 2018

Contributor

First thought was to save some hash allocations, but agree, no point in chasing those several dozens additional hashes, also if we decide to make some default-for-trait-objects they can be stored in the same hash under special keys.

@@ -6,6 +6,6 @@
specify "RSpec integration", :aggregate_failures do
output = run_rspec('factory_default')
expect(output).to include("5 examples, 0 failures")
expect(output).to include(" examples, 0 failures")

This comment has been minimized.

@palkan

This comment has been minimized.

@Vasfed

Vasfed Jun 28, 2018

Contributor

here we are interested more in the spec passing, thought it is redundant to update test count each time the fixture spec has a test added, restored to explicit count

@Vasfed

This comment has been minimized.

Contributor

Vasfed commented Jun 28, 2018

@palkan please see new commit

if traits && !traits.empty?
return false if FactoryDefault.preserve_traits || @store_preserve_traits[name]
return nil if FactoryDefault.preserve_traits || record[:preserve_traits]

This comment has been minimized.

@palkan

palkan Jun 28, 2018

Owner

Unnecessary nil

This comment has been minimized.

@Vasfed

Vasfed Jun 28, 2018

Contributor

Enabled rubocop to check for that

record[:object]
end
def exists?(name, traits = nil)

This comment has been minimized.

@palkan

palkan Jun 28, 2018

Owner

Do we use exists? anywhere?

This comment has been minimized.

@Vasfed

Vasfed Jun 28, 2018

Contributor

No, we don't, can be safely removed - left it for compatibility. It was public and appeared in generated documentation, someone could have used it for some test suite debugging. (sure I doubt it too)

This comment has been minimized.

@palkan

palkan Jun 28, 2018

Owner

Let's remove then

This comment has been minimized.

@Vasfed

Vasfed Jun 28, 2018

Contributor

Ok, done

Vasfed added some commits Jun 28, 2018

@palkan

palkan approved these changes Jun 28, 2018

@palkan palkan merged commit 5d9c543 into palkan:master Jun 28, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@palkan

This comment has been minimized.

Owner

palkan commented Jun 28, 2018

Thanks! Great job!

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