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

WIP: Support for JRuby-9000 #3792

Closed
wants to merge 8 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@kennethkalmer
Contributor

kennethkalmer commented Feb 13, 2015

Will bring in the combined efforts of previous attempts to get activeadmin running on jruby-9000, slated to be the next major JRuby release.

  • Get dependencies installed
  • Get the tests running
  • Fix any test failures
  • Hook up to CI
  • Prosper
chore: added jruby-head (allowing failures) to travis
Also increase allowed memory usage
Show outdated Hide outdated Gemfile
gem 'redcarpet' # Markdown implementation (for yard)
gem 'yard' # Documentation generator
gem 'redcarpet', platforms: :mri # Markdown implementation (for yard)
gem 'kramdown', platforms: :jruby # Markdown implementation (for yard)

This comment has been minimized.

@timoschilling

timoschilling Feb 13, 2015

Member

please bring all comments of all blocks into one line

@timoschilling

timoschilling Feb 13, 2015

Member

please bring all comments of all blocks into one line

This comment has been minimized.

@kennethkalmer

kennethkalmer Feb 13, 2015

Contributor

across the entire Gemfile? Just making sure before I do

@kennethkalmer

kennethkalmer Feb 13, 2015

Contributor

across the entire Gemfile? Just making sure before I do

This comment has been minimized.

@timoschilling

timoschilling Feb 13, 2015

Member

I think across the entire Gemfile is a good idea

@timoschilling

timoschilling Feb 13, 2015

Member

I think across the entire Gemfile is a good idea

@kennethkalmer

This comment has been minimized.

Show comment
Hide comment
@kennethkalmer

kennethkalmer Feb 13, 2015

Owner

This is tripping me up a bit, it seems something is pulling in Test::Unit's AutoRunner the whole time. Even generating the rails application as part of rake setup has output from the autorunner.

I got this "fix" from rspec/rspec-rails#1171 but I'd be happier if we could not have this...

I wonder if @headius has seen a situation before where the autorunner kicks in on JRuby in this way...?

Owner

kennethkalmer commented on 1874d75 Feb 13, 2015

This is tripping me up a bit, it seems something is pulling in Test::Unit's AutoRunner the whole time. Even generating the rails application as part of rake setup has output from the autorunner.

I got this "fix" from rspec/rspec-rails#1171 but I'd be happier if we could not have this...

I wonder if @headius has seen a situation before where the autorunner kicks in on JRuby in this way...?

@brennovich

This comment has been minimized.

Show comment
Hide comment
@brennovich

brennovich Feb 15, 2015

Contributor

I'll try to help with this issue :)

Although I have a concern: will Rails 3.2.X be supported? If so, I don't think we'll be able to accomplish that.

Besides that I'll start with Rails 4.2.0.

Contributor

brennovich commented Feb 15, 2015

I'll try to help with this issue :)

Although I have a concern: will Rails 3.2.X be supported? If so, I don't think we'll be able to accomplish that.

Besides that I'll start with Rails 4.2.0.

@brennovich

This comment has been minimized.

Show comment
Hide comment
@brennovich

brennovich Feb 15, 2015

Contributor

Well, well all unit specs seems to be passing, I needed to increase heap size though

screenshot 2015-02-15 19 47 44

Contributor

brennovich commented Feb 15, 2015

Well, well all unit specs seems to be passing, I needed to increase heap size though

screenshot 2015-02-15 19 47 44

@kennethkalmer

This comment has been minimized.

Show comment
Hide comment
@kennethkalmer

kennethkalmer Feb 16, 2015

Contributor

@brennovich thanks! I bumped the heap to 1024 and it all passed, but on Travis it timed out after 50 mins :(

I guess we need to find the source of Test::Unit's AutoRunner kicking in next.

There are also some specs using ObjectSpace which I expected to fail off the bat, but either they work or they simply haven't run yet.

Contributor

kennethkalmer commented Feb 16, 2015

@brennovich thanks! I bumped the heap to 1024 and it all passed, but on Travis it timed out after 50 mins :(

I guess we need to find the source of Test::Unit's AutoRunner kicking in next.

There are also some specs using ObjectSpace which I expected to fail off the bat, but either they work or they simply haven't run yet.

@timoschilling

This comment has been minimized.

Show comment
Hide comment
@timoschilling

timoschilling Mar 12, 2015

Member

can you please move jruby from the allow_failures list to the normal list. So that this PR is listet as failing in GH until you are done.

Member

timoschilling commented Mar 12, 2015

can you please move jruby from the allow_failures list to the normal list. So that this PR is listet as failing in GH until you are done.

@kennethkalmer

This comment has been minimized.

Show comment
Hide comment
@kennethkalmer

kennethkalmer Mar 12, 2015

Contributor

@timoschilling not yet, I've been soliciting feedback to help see this through. That said, let me put in some time today to merge in the mainline branches and run tests again. Thanks for the push to get his merged!

Contributor

kennethkalmer commented Mar 12, 2015

@timoschilling not yet, I've been soliciting feedback to help see this through. That said, let me put in some time today to merge in the mainline branches and run tests again. Thanks for the push to get his merged!

@timoschilling

This comment has been minimized.

Show comment
Hide comment
@timoschilling

timoschilling Mar 12, 2015

Member

It's ok that you still need time for this PR. I only want to see the true CI status in the PR List like it is on #3742
pull_requests_ _activeadmin_activeadmin

Member

timoschilling commented Mar 12, 2015

It's ok that you still need time for this PR. I only want to see the true CI status in the PR List like it is on #3742
pull_requests_ _activeadmin_activeadmin

kennethkalmer and others added some commits Mar 13, 2015

Merge pull request #1 from brennovich/jruby-compat
Enable ObjectSpace emulation on travis for jRuby
@@ -107,6 +107,9 @@ def with_translation(translation)
require 'rspec/rails'
# prevent Test::Unit's AutoRunner from executing during RSpec's rake task on JRuby

This comment has been minimized.

@houndci-bot

houndci-bot Mar 22, 2015

Line is too long. [82/80]

@houndci-bot

houndci-bot Mar 22, 2015

Line is too long. [82/80]

@deivid-rodriguez

This comment has been minimized.

Show comment
Hide comment
@deivid-rodriguez

deivid-rodriguez Jul 16, 2015

Member

I tried activeadmin with JRuby today. It's working.

I rebased this PR and got only a couple of test failures:

Failing Scenarios:
cucumber features/index/batch_actions.feature:3 # Scenario: Use default (destroy) batch action
cucumber features/index/batch_actions.feature:40 # Scenario: Use default (destroy) batch action on a nested resource

The functionality they test actually works, so it seems like a bug / misuse of capybara / poltergeist.

Member

deivid-rodriguez commented Jul 16, 2015

I tried activeadmin with JRuby today. It's working.

I rebased this PR and got only a couple of test failures:

Failing Scenarios:
cucumber features/index/batch_actions.feature:3 # Scenario: Use default (destroy) batch action
cucumber features/index/batch_actions.feature:40 # Scenario: Use default (destroy) batch action on a nested resource

The functionality they test actually works, so it seems like a bug / misuse of capybara / poltergeist.

@deivid-rodriguez

This comment has been minimized.

Show comment
Hide comment
@deivid-rodriguez

deivid-rodriguez Jul 16, 2015

Member

If I add the @javascript tag on top of those scenarios, those assertions pass, but I get other failures because the assertions are not supported by the poltergeist driver.

Member

deivid-rodriguez commented Jul 16, 2015

If I add the @javascript tag on top of those scenarios, those assertions pass, but I get other failures because the assertions are not supported by the poltergeist driver.

@timoschilling

This comment has been minimized.

Show comment
Hide comment
@timoschilling

timoschilling Jul 17, 2015

Member

@deivid-rodriguez 👍 can you continue the work on that?

Member

timoschilling commented Jul 17, 2015

@deivid-rodriguez 👍 can you continue the work on that?

@deivid-rodriguez

This comment has been minimized.

Show comment
Hide comment
@deivid-rodriguez

deivid-rodriguez Jul 17, 2015

Member

I'm on it, I'll open a PR superseding this one. Is that ok?

Member

deivid-rodriguez commented Jul 17, 2015

I'm on it, I'll open a PR superseding this one. Is that ok?

@brennovich

This comment has been minimized.

Show comment
Hide comment
@brennovich

brennovich Jul 17, 2015

Contributor

@deivid-rodriguez even the memory leak specs are passing?

Can you keep with this PR? I think the history here may be important in the future ;)

Contributor

brennovich commented Jul 17, 2015

@deivid-rodriguez even the memory leak specs are passing?

Can you keep with this PR? I think the history here may be important in the future ;)

@deivid-rodriguez

This comment has been minimized.

Show comment
Hide comment
@deivid-rodriguez

deivid-rodriguez Jul 17, 2015

Member

Everything is passing. But I haven't tried jruby-head, only jruby.9.0.0.0.rc2. I'll let travis do that.

You mean opening a PR at kennethkalmer/activeadmin?

Member

deivid-rodriguez commented Jul 17, 2015

Everything is passing. But I haven't tried jruby-head, only jruby.9.0.0.0.rc2. I'll let travis do that.

You mean opening a PR at kennethkalmer/activeadmin?

@deivid-rodriguez

This comment has been minimized.

Show comment
Hide comment
@deivid-rodriguez

deivid-rodriguez Jul 17, 2015

Member

@brennovich Note that I am keeping history, I'm just doing it in a new branch in my own fork.

Member

deivid-rodriguez commented Jul 17, 2015

@brennovich Note that I am keeping history, I'm just doing it in a new branch in my own fork.

@brennovich

This comment has been minimized.

Show comment
Hide comment
@brennovich

brennovich Jul 17, 2015

Contributor

Awesome!

Humm not sure, I think you're right, is better to open up a new one, just make sure to comment the new PR number here in the comments :)

Thanks o/

Contributor

brennovich commented Jul 17, 2015

Awesome!

Humm not sure, I think you're right, is better to open up a new one, just make sure to comment the new PR number here in the comments :)

Thanks o/

@deivid-rodriguez

This comment has been minimized.

Show comment
Hide comment
@deivid-rodriguez

deivid-rodriguez Jul 17, 2015

Member

Opening a sub-PR is good because commit authors are kept, but just rebasing my work on top of a copy of @kennethkalmer's branch seems easier (and already done).

Member

deivid-rodriguez commented Jul 17, 2015

Opening a sub-PR is good because commit authors are kept, but just rebasing my work on top of a copy of @kennethkalmer's branch seems easier (and already done).

@brennovich

This comment has been minimized.

Show comment
Hide comment
@brennovich

brennovich Jul 17, 2015

Contributor

👍

Contributor

brennovich commented Jul 17, 2015

👍

@deivid-rodriguez deivid-rodriguez referenced this pull request Jul 17, 2015

Merged

Jruby compat #4032

@deivid-rodriguez

This comment has been minimized.

Show comment
Hide comment
@deivid-rodriguez

deivid-rodriguez Jul 17, 2015

Member

Done! And the commit authors are preserved... I love git. 😄

Let's see how the build goes.

Member

deivid-rodriguez commented Jul 17, 2015

Done! And the commit authors are preserved... I love git. 😄

Let's see how the build goes.

@timoschilling

This comment has been minimized.

Show comment
Hide comment
@timoschilling
Member

timoschilling commented Jul 30, 2015

closed #4032

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