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

Getter method for strategy (to help with debugging) #511

Open
rlue opened this issue Nov 16, 2017 · 4 comments
Open

Getter method for strategy (to help with debugging) #511

rlue opened this issue Nov 16, 2017 · 4 comments

Comments

@rlue
Copy link
Contributor

rlue commented Nov 16, 2017

It appears that DatabaseCleaner does not currently support a getter method for determining which strategy it currently employs:

>> DatabaseCleaner.strategy
NoMethodError: undefined method `strategy' for DatabaseCleaner:Module
Did you mean?  strategy=
from (pry):1:in `block (2 levels) in <top (required)>'

It would be nice to have this just so I could confirm, in my debugger, that I have DatabaseCleaner set up the way I had intended.

Is there another way to do this that I've missed?

@etagwerker
Copy link
Member

etagwerker commented Nov 22, 2017

@rlue I think it's a good idea but it's not a trivial change because of this:

def strategy=(stratagem)
connections.each { |connect| connect.strategy = stratagem }
remove_duplicates
end

If your application is using more than one connection, then that could return an Array (or a Hash?)

Are you interested in working on a PR for this?

@rlue
Copy link
Contributor Author

rlue commented Nov 22, 2017

I am, but it's going to be a while before I have time to familiarize myself with the codebase enough to know what the implications of my changes will be.

I'm not sure I understand — the ::strategy= method appears to unconditionally set the strategy on all connections. Is there another way that an individual connection's strategy might be changed? Otherwise, wouldn't we expect all connections to share the same strategy?

@rlue
Copy link
Contributor Author

rlue commented Jan 2, 2018

@etagwerker I finally got around to taking a look at the code. Here's my understanding — please correct me if I'm mistaken:

  • Each “connection” is an element of the @connections instance variable on the DatabaseCleaner module, and
  • These connections are instances of the DatabaseCleaner::Base class.

What would you think of adding the #strategy getter method on the DatabaseCleaner::Base class? That way, DatabaseCleaner[].strategy would return the strategy for the default connection, and DatabaseCleaner[:mongoid].strategy would return the strategy for that one. Then, for some syntactic sugar, we could add a DatabaseCleaner.strategy method that aliases DatabaseCleaner[].strategy.

On the other hand, judging from the comments, it looks like you plan on doing a deeper redesign of the class hierarchy database_cleaner. Thoughts?

@rlue
Copy link
Contributor Author

rlue commented Jan 6, 2018

Sorry, disregard my last comment. 1) I hadn't noticed that DatabaseCleaner::Base#strategy is already defined, and 2) I misunderstood how DatabaseCleaner::[] works.

I'm submitting a PR according to your previous suggestion.

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

No branches or pull requests

2 participants