v2.0.0.beta #620

botandrose · 2020-02-01T23:41:03Z

Oh boy, here it is! This branch contains all the big changes needed for the v2.0.0.beta release:

Removal of all adapters
Removal of all deprecated code
Drop support for EOL Rubies and add support for current Rubies
Rename gem to database_cleaner-core
Add new database_cleaner metagem that depends on database_cleaner-active_record

Not ready to merge this yet... I want to get all the adapter repos into a good spot first. But I figured its good to get all this out in the open nice and early, so that people can take a look and give any feedback they might have!

Exciting times for database_cleaner!

botandrose · 2020-02-02T17:52:38Z

@etagwerker @JonRowe All right! I think this PR + DatabaseCleaner/database_cleaner-active_record#3 represent the current vision for v2.0.0. Will y'all take a look and let me know what you think?

When we agree that this all looks good, I'll finish up the other adapters and release v2.0.0.beta versions of everything for wider testing.

Very exciting to finally escape from dependency hell!

JonRowe · 2020-02-02T20:30:44Z

spec/database_cleaner/configuration_spec.rb

@@ -93,6 +86,9 @@ def stub_cleaners(array)
    end

    it "should retrieve a db rather than create a new one" do
+      stub_const "DatabaseCleaner::ActiveRecord", Module.new
+      stub_const "DatabaseCleaner::ActiveRecord::Truncation", Class.new


You could use object_double here to provide greater reliability.

Hmm, I having a hard time getting this to work with object_double, but I may be doing it wrong! Can you say a bit more about how this might work?

Sorry I meant class_double.

Oh, okay! But I can't seem to get that working either 🤣

Can you say a bit more about how class_double could improve this spec? Sorry I'm so dense... I used to think I was pretty darn knowledgeable about RSpec!

class_double("DatabaseCleaner::ActiveRecord::Truncation").as_stubbed_const gives you a double which will only allow you to stub/expect methods that exist on the class, its a verifying double :)

Ohhh, I see what you're getting at. I actually don't think that would be helpful here, because the database_cleaner-active_record gem isn't loaded at all! This suite is testing core in isolation from any of the adapters. That's why we need to stub these constants so that the strategy lookup code has something to find when we say DatabaseCleaner[:active_record].strategy = :truncation.

So, I don't think a verifying double would add much value, because the only methods being called on these constants is const_get on DC::AR to get DC::AR::Truncation, and .new on DC::AR::Truncation.

Now that I write this, it occurs to me that this situation could probably be improved by using an obviously fake set of constants, so that its clear that the implementation doesn't matter.

What do you think?

They behave like normal doubles if the class isn't loaded, they offer an extra level of protection when they are.

Wow, once again, I didn't know that! Very cool. I'm convinced. I'll give it a shot!

Okay, pushed a new commit using class_double that I hope is making good use of it. Thanks for being a great teacher!

JonRowe · 2020-02-02T20:33:02Z

Gemfile.lock

@@ -1,153 +1,20 @@
 PATH
  remote: .
  specs:
-    database_cleaner (1.8.2)


Typically speaking Gemfile.lock shouldn't be committed, maybe take this opportunity to remove it?

Can you explain the value in gitignoring Gemfile.lock in library repos? I've never been entirely clear on that. I'm guessing its that future test runs will pick up newly released versions of dependencies, giving us a heads-up if one of them fails?

Personally, I've always weighed this against build/failure reproducibility across development instances and found the latter more compelling, but I'd love to learn something new here. What are your views?

It prevents builds from staying in sync with reality. Your gain stability in your builds by locking to a period of time rather than versions. If you have specific versions that you need for a build to pass they should be in your gemspec, or if you need some custom magic in your Gemfile.

Someone who consumes the library will not get the versions specified in your Gemfile or Gemfile.lock no matter how they get the library, so you end up not seeing failures that affect them until someone bothers to open an issue.

@JonRowe Ah, that makes sense, and its a particularly relevant argument, considering the amount of effort this library is currently expending to address exactly that situation, i.e. tied to a 2010 reality!

I wonder though, does the existence of dependabot completely change this situation? Now that its on the job, do we get the best of both worlds by leaving Gemfile.lock checked in? Developers of the library don't suffer from works-on-my-machine, and dependabot ensures that we stay in sync with reality?

Dependabot isn't up to that yet. It too often misses things because theres no security implication, or ignores things it can't update.

Ah, well, that's a bummer. Maybe some day. I'll remove the .lock files from git before the release.

README.markdown

database_cleaner-core.gemspec

lib/database_cleaner-core.rb

JonRowe · 2020-02-02T21:47:12Z

spec/database_cleaner/base_spec.rb

@@ -231,23 +95,29 @@ module DatabaseCleaner
    end

    describe "clean_with" do
-      # FIXME hacky null strategy
-      # because you can't pass a NullStrategy to #clean_with
+      subject { described_class.new(:active_record) }


I know this to make the existing specs work, but if you're modernising here, we, the rspec team, recommend you at least use named subject, and we don't really recommend using described_class (as it requires constants to be loaded before use) but that might be something for a later PR.

TIL about named subjects! 😃 I'll give that a go.

Also, if you ever want to make a pass through the test suites, they all need a lot of work! A decade of being tied to the 2010-era gem ecosystem, while the world moves on, has necessarily yielded a paradigm of testing implementation, not behavior. :/ I've already done some work on the DC::AR adapter gem's suite, but once this PR lands, we're finally free to modernize things!

Okay, I've pushed a new commit that names all custom subjects, and removes usage of described_class, as well as a number of other cleanups. WDYT?

database_cleaner-core.gemspec

spec/database_cleaner/base_spec.rb

lib/database_cleaner/core.rb

botandrose · 2020-02-04T19:52:07Z

@JonRowe @etagwerker Okay, I have removed the [WIP] from the PR title, because I think this is ready to go! There are a few issues that Jon brought up that remain unresolved, like exactly what to do with Gemfile.lock & dependabot, but I don't think those or anything else is blocking a v2.0.0.beta release! We can figure those out in the space between the beta and full release.

I have also created v2.0.0.beta PRs for the five adapters I think we should continue to support (the rest I think we can leave at 1.8):

I know both of you are very busy, so any objections to me merging these PRs and cutting v2.0.0.beta releases in, let's say, three days?

JonRowe · 2020-02-04T20:01:12Z

I have no objections, but @etagwerker should have a say

etagwerker · 2020-02-05T13:52:12Z

@botandrose @JonRowe looking into it. Thanks for your hard work! ❤️

etagwerker

@botandrose Looks great! Just added a few comments.

.travis.yml

README.markdown

database_cleaner-core.gemspec

etagwerker · 2020-02-06T15:43:38Z

lib/database_cleaner/base.rb

@@ -48,8 +45,7 @@ def strategy
    attr_reader :orm

    def orm=(desired_orm)
-      @orm = (desired_orm || :autodetect).to_sym
-      @orm = @orm_autodetector.orm if @orm == :autodetect
+      @orm = desired_orm && desired_orm.to_sym


@botandrose I don't understand this line. Wouldn't this be the same?

Suggested change

@orm = desired_orm && desired_orm.to_sym

@orm = desired_orm.to_sym

Nil raises an error if you do that.

Hmm yeah. I've added a test case to the spec to ensure that passing nil doesn't cause it to crash.

Although, now I'm thinking it should raise NoORMDetected like DatabaseCleaner[nil] does. What do you guys think?

Probably should rename that exception, too, while we're at it, now that there's no more orm detection!

I mean I think its far to raise an ArgumentError here at least...

Yeah, good call. That sounds like the right move to me. Replace NoORMDetected with ArgumentError, and raise it in both cases.

Digging into this issue, I began to wonder if this is an opportunity to expose NullStrategy? For example:

DatabaseCleaner[nil].clean # => Clean with NullStrategy DatabaseCleaner.orm = nil # => Set to NullStrategy DatabaseCleaner.strategy = nil # => Set to NullStrategy

This is getting a little out of scope of this PR, but what do y'all think?

I think the likely hood of someone doing that intentionally is lower than someone making a mistake and passing nil unintentionally, you'd still want an error or a warning somewhere even if you did that.

spec/database_cleaner/configuration_spec.rb

botandrose · 2020-02-14T00:37:50Z

@etagwerker @JonRowe I'm feeling pretty good. Any objection to merging this PR (and adapter PRs) into master and releasing a v2.0.0.beta series? Unresolved conversations here could be split off into their own issues to resolve before v2.0.0 final so we don't lose track of them. Yea or nay?

JonRowe · 2020-02-14T10:06:02Z

No blocking objections

botandrose · 2020-02-14T20:17:09Z

Okay, I think everything has been resolved, now! Please let me know if not. Merging this into master.

JonRowe · 2020-02-15T09:28:54Z

Great work 🙌

@botandrose

The library is now splitted into multiple gems, depending on the ORM / database you use. Changes since 1.7.0: ``` == 2.0.1 2021-02-04 == Bugfixes * Regression: allow_remote_database_url and url_allowlist not working anymore: DatabaseCleaner/database_cleaner#671 == 2.0.0 2021-01-31 === Changes * Rename `url_whitelist` to `url_allowlist` * Allowlist now supports regular expressions * Fixed Ruby 2.7 deprecation warnings === Breaking changes * Failed checks against the allowlist now raise `UrlNotAllowed` rather than `NotWhitelistedUrl` == 2.0.0.beta2 2020-05-30 === Features * New API for ORM Adapter gems: DatabaseCleaner/database_cleaner#644 === Breaking changes * Rename :connection configuration option to :db for consistency: DatabaseCleaner/database_cleaner#650 * Remove all #orm= setter methods: https://github.com/DatabaseCleaner/database_cleaner/pull/643/files * drop support for Ruby 2.4 which is EOL as of 2020-03-31: DatabaseCleaner/database_cleaner#635 == 2.0.0.beta 2020-04-05 === Breaking changes * Replace old shared RSpec examples with new "database_cleaner adapter" example: DatabaseCleaner/database_cleaner#629 * split gem into database_cleaner-core and database_cleaner metagem. * Support Ruby versions 2.4, 2.5, 2.6, and 2.7, and drop support for older Rubies. * remove all deprecated code and get the specs passing again. * Split off all adapter gems into their own repos: DatabaseCleaner/database_cleaner#620 == 1.99.0 2021-01-31 == Changes * Remove unnecessary dependency on database_cleaner-mongo from database_cleaner-mongoid: @botandrose * Enable the :cache_tables option for the mongo truncation strategy, and default to true: DatabaseCleaner/database_cleaner#646" * Introduce deletion aliases for truncation strategies for mongo, mongoid, and redis adapters. DatabaseCleaner/database_cleaner#654 * Add new :db orm configuration key, for consistency with #db and #db=. DatabaseCleaner/database_cleaner#649 == Deprecations * Deprecate all #orm= setter methods: DatabaseCleaner/database_cleaner#643 * Deprecate non-functional :reset_ids option in ActiveRecord truncation strategy: DatabaseCleaner/database_cleaner#559 * Deprecate mongo truncation's `:cache_tables => true` option in favor of `false`, to prep for caching removal in v2.0: DatabaseCleaner/database_cleaner#646" * Deprecate redis truncation's #url method in favor of #db: @botandrose * Deprecate mongo, mongoid, and redis truncation strategies in favor of deletion. DatabaseCleaner/database_cleaner#654 * Deprecate :connection and :model configuration options in favor of :db for consistency: DatabaseCleaner/database_cleaner#650 == Bugfixes * Fix deprecation warning about `DatabaseCleaner.connections` to recommend a better alternative: DatabaseCleaner/database_cleaner#656 == 1.8.5 2020-05-04 === Bug Fixes * Fix :mongo strategy: DatabaseCleaner/database_cleaner#645 == 1.8.4 2020-04-02 === Bug Fixes * Fix false positive deprecation warnings on Windows: DatabaseCleaner/database_cleaner#633 == 1.8.3 2020-02-18 === Bug Fixes * Fix performance issue of DatabaseCleaner::Base#orm_module: DatabaseCleaner/database_cleaner#625 == 1.8.2 2020-02-01 === Bug Fixes * Fix database_cleaner-ohm autodetected adapter loading: DatabaseCleaner/database_cleaner#619 * Fix database_cleaner-mongo_mapper autodetected adapter loading: @botandrose * Fix database_cleaner-mongoid autodetected adapter loading: DatabaseCleaner/database_cleaner#617 * Exclude ar_internal_metadata from truncation on Rails 5: DatabaseCleaner/database_cleaner#588 === Changes * Deprecate ohm adapter: DatabaseCleaner/database_cleaner#619 == 1.8.1 2020-01-30 === Bug Fixes * Remove undeclared active_support dependency: DatabaseCleaner/database_cleaner#612 == 1.8.0 2020-01-29 === Bug Fixes * Fix MySQL deprecation warnings with Rails 5: DatabaseCleaner/database_cleaner#574 * Fix MySQL truncation with `pre_count: true`: DatabaseCleaner/database_cleaner#498 * Fix primary key sequence resetting in Sequel with Postgres and SQLite: https://github.com/DatabaseCleaner/database_cleaner/pull/538/files * ActiveRecord truncation adapter doesn't work with Oracle: DatabaseCleaner/database_cleaner#542 === Changes * Extract ORM adapters into gems: DatabaseCleaner/database_cleaner#560 * Allow postgres:///dbname as a local url: DatabaseCleaner/database_cleaner#569 * Add an optional URL whitelist safeguard: DatabaseCleaner/database_cleaner#526 * Add `local` tld to safeguard check: DatabaseCleaner/database_cleaner#547 * Speed up ActiveRecord deletion strategy: DatabaseCleaner/database_cleaner#534 * Consider `sqlite:` database urls to be local: DatabaseCleaner/database_cleaner#529 ```

botandrose-machine added 7 commits February 2, 2020 08:38

begin development on v2.0.0.beta.

2da1f49

split all adapter gems into their own repos.

e658c11

add byebug for debugging.

0cf7816

remove all deprecated code and get the specs passing again.

a593621

drop support for older rubies, and add support for current rubies.

eb80a78

modernize gemspec.

4d50d88

don't run cucumber tests in this repo, at least not yet.

22192c8

botandrose force-pushed the v2.0.0.beta branch from 706c99b to 0b3491b Compare February 2, 2020 16:40

botandrose requested review from etagwerker and JonRowe February 2, 2020 17:49

JonRowe reviewed Feb 2, 2020

View reviewed changes

botandrose-machine added 3 commits February 2, 2020 15:21

split gem into database_cleaner-core and database_cleaner metagem.

fa72e43

RIP mysql1 adapter.

00c212f

modernize DC::Base specs.

504b9de

botandrose force-pushed the v2.0.0.beta branch from 0b3491b to 504b9de Compare February 2, 2020 23:40

bump latest release version on README.

e90c54b

JonRowe reviewed Feb 3, 2020

View reviewed changes

database_cleaner-core.gemspec Outdated Show resolved Hide resolved

JonRowe reviewed Feb 3, 2020

View reviewed changes

spec/database_cleaner/base_spec.rb Outdated Show resolved Hide resolved

fix gem loading situation.

2181127

JonRowe reviewed Feb 3, 2020

View reviewed changes

lib/database_cleaner/core.rb Outdated Show resolved Hide resolved

botandrose-machine and others added 3 commits February 3, 2020 09:23

remove extra line endings.

9c88241

reduce usage of bare subject on advice of rspec core team.

5ab507c

mark certain adapters as discontinued.

33b8bb8

botandrose changed the title ~~[WIP] v2.0.0.beta~~ v2.0.0.beta Feb 4, 2020

use class_double for a bit of extra reliability.

3f4d395

etagwerker reviewed Feb 6, 2020

View reviewed changes

botandrose-machine added 3 commits February 7, 2020 10:49

add Micah Geisel to list of gem authors.

422d762

test nil case for DC::Base#orm=.

7410730

core no longer knows what a valid adapter is.

aa52404

botandrose-machine added 2 commits February 14, 2020 12:13

raise ArgumentError when DC[nil] is invoked or DC.orm = nil.

59c54ab

rm Gemfile.lock from version control.

d59d53a

botandrose merged commit d6f8bb1 into master Feb 14, 2020

botandrose deleted the v2.0.0.beta branch February 14, 2020 20:19

This was referenced Mar 12, 2021

Bump database_cleaner from 1.99.0 to 2.0.1 openfoodfoundation/openfoodnetwork#7081

Merged

Bump database_cleaner from 1.8.5 to 2.0.1 ministryofjustice/laa-apply-bot#233

Closed

Bump database_cleaner from 1.99.0 to 2.0.1 Matt-Yorkley/openfoodnetwork#15

Merged

	@orm = desired_orm && desired_orm.to_sym
	@orm = desired_orm.to_sym

v2.0.0.beta #620

v2.0.0.beta #620

Conversation

botandrose commented Feb 1, 2020

botandrose commented Feb 2, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

botandrose commented Feb 4, 2020 • edited Loading

JonRowe commented Feb 4, 2020

etagwerker commented Feb 5, 2020

etagwerker left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

botandrose commented Feb 14, 2020

JonRowe commented Feb 14, 2020

botandrose commented Feb 14, 2020

JonRowe commented Feb 15, 2020

botandrose commented Feb 4, 2020 •

edited

Loading