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

[Fix #80] Ability to preserve traits #83

Merged
merged 4 commits into from Jun 28, 2018

Conversation

@Vasfed
Copy link
Contributor

@Vasfed 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
Collaborator

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
Collaborator

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
Author 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

palkan Jun 28, 2018
Collaborator

?

This comment has been minimized.

@Vasfed

Vasfed Jun 28, 2018
Author 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
Copy link
Contributor Author

@Vasfed 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
Collaborator

Unnecessary nil

This comment has been minimized.

@Vasfed

Vasfed Jun 28, 2018
Author 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
Collaborator

Do we use exists? anywhere?

This comment has been minimized.

@Vasfed

Vasfed Jun 28, 2018
Author 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
Collaborator

Let's remove then

This comment has been minimized.

@Vasfed

Vasfed Jun 28, 2018
Author Contributor

Ok, done

Vasfed added 2 commits Jun 28, 2018
@palkan
palkan approved these changes Jun 28, 2018
@palkan palkan merged commit 5d9c543 into test-prof:master Jun 28, 2018
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@palkan
Copy link
Collaborator

@palkan palkan commented Jun 28, 2018

Thanks! Great job!

@palkan
Copy link
Collaborator

@palkan palkan commented Aug 2, 2019

Hey @Vasfed!

Could you please send me your actual email address to palkan@evl.ms? We plan to send some goodies to the Cult participants)

@Vasfed
Copy link
Contributor Author

@Vasfed Vasfed commented Aug 2, 2019

@palkan Hey! Sent email, e064395fd8a88d6d6c887132d7e1e39035e841a12fc6c5f28cc348ffb4c62b16

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants