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

Skip test which is broken on jruby #12816

Merged
merged 1 commit into from
Nov 9, 2013
Merged

Skip test which is broken on jruby #12816

merged 1 commit into from
Nov 9, 2013

Conversation

gaurish
Copy link
Contributor

@gaurish gaurish commented Nov 8, 2013

This test is broken from quite a while & is expected to remain broken as
encoding issues are hardest to fix in JRuby. so lets skip this test for
now.

Related bug report: http://jira.codehaus.org/browse/JRUBY-7192
#11700

This test is broken from quite a while & is expected to remain broken as
encoding issues are hardest to fix in JRuby. so lets skip this test for
now
rafaelfranca added a commit that referenced this pull request Nov 9, 2013
Skip test which is broken on jruby
@rafaelfranca rafaelfranca merged commit 3cc64df into rails:master Nov 9, 2013
@@ -37,6 +37,10 @@ def test_serves_static_index_file_in_directory
end

def test_served_static_file_with_non_english_filename
if RUBY_ENGINE == 'jruby '
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why we don't use jruby_skip here please ? :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jruby_skip is only available in ActiveSupport as its defined in abtract_unit.rb. And caused an NoMethodError exception when I tried to use it here.

if you know a way that jruby_skip can be used in entire codebase, kindly let me know. I would happy to submit a PR

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like you are right ; currently it's not possible (in a clean way I mean) to load Active Support's abstract unit in Action Pack's. I thought we could move it to ActiveSupport::TestCase but I don't think this should be public API. What do you think @rafaelfranca ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is what I think

  • for code reuse. we can either inherit from common class(ActiveSupport::Testing::Assertions?) which would make this a public api OR make it a module which can be included in abstract units of all rails components. But making a module for just for 2 helper methods would be overkill.
  • Alternatively, if some code duplication is okay, we can paste following line in abstract unit of AS & AP
%w[jruby rbx].each do |runtime|
  define_method("#{runtime}_skip") {|msg=''| skip msg if RUBY_ENGINE == runtime}
end

this would generate jruby_skip & rbx_skip methods. IMHO, this is the simplest approach & what I would like to do. so let me know if its okay, I will submit a PR.

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

Successfully merging this pull request may close these issues.

None yet

3 participants