Support ActiveSupport 4.1 about html-pipeline HOT 8 CLOSED

I created an issue in rails/rails.

from html-pipeline.

So much for clean OO design; the attitude expressed in that Rails bug is pretty brusque. I suppose anybody in this position is expected to fork AS 4.0.5 or 3.2.18 and cherry-pick from upstream forever. 👎

from html-pipeline.

Since ActiveSupport won't change their gemspec, we must explore other options.

1. Refactor test suite to use minitest gem instead of stdlib test/unit
2. Before the test helper requires 'test/unit', we could explicitly require the stdlib 'minitest/unit', so test/unit does not require the minitest gem.

Option 2 might be OK, but the code might be hacky.

Option 1 didn't seem reasonable, but Ruby 2.2.0 is removing stdlib test!

From ruby/NEWS:

• lib/test/*/.rb
  • Removed because it conflicts to minitest 5, and it was just an wrapper
    of minitest 4. [Feature #9711]

Maybe option 1 is the right choice... Thoughts?

cc @jch @rsanheim

from html-pipeline.

@simeonwillbanks Agree with your assessment of both options. 👍 for Option 1

from html-pipeline.

I love minitest, but hate that it's a gem rather than in the stdlib. That's orthogonal to this discussion though. I'm 👍 to seeing a option 1 PR with minitest.

from html-pipeline.

Feature #9711 - "Remove test-unit and minitest from stdlib" details the removal.

Feature #9852 - "How to bundle test-unit2 and minitest5" discusses what's next.

It appears the stdlib minitest is and will be the minitest gem. Ruby trunk imports the minitest gem into the stdlib.

That said, I still think we should bundle the minitest gem and proceed with the refactor because ActiveSupport is depending upon minitest via development_dependency. The ActiveSupport minitest version doesn't match the stdlib version, and this could continue. We'll have less issues matching ActiveSupport rather than the stdlib. 😄

Thanks for the input @jdickey and @jch. I'll start a Pull Request.

from html-pipeline.

Is ActiveSupport really a runtime dependency still? I know the instrumentation stuff is expecting something that implements ActiveSupport::Notifications, but it doesn't seem to actually depend on it inheriting from that. Is there something else I'm missing outside of the tests?

I made a branch that removes ActiveSupport entirely (replacing the HTML fragment comparison functionality with something based on lorax) and all the unit tests still pass on 1.9.3 at least. (Happy to submit this as a PR if you like.)

ActiveSupport is just kind of a large and opinionated dependency to add to non-Rails based apps if it's not strictly necessary.

from html-pipeline.

e00c3c9 fixes this issue.

from html-pipeline.