Skip to content

Conversation

@mrkn
Copy link
Member

@mrkn mrkn commented Oct 29, 2018

This commit fixes the following failure:

  1) Failure:
Fiddle::TestImport#test_no_message_with_debug [/Users/mrkn/src/github.com/ruby/fiddle/test/fiddle/test_import.rb:152]:

1. [2/2] Assertion for "stderr"
   | <[]> expected but was
   | <["Exception `NameError' at /Users/mrkn/.rbenv/versions/2.5.1/lib/ruby/2.5.0/fiddle/import.rb:157 - uninitialized constant Fiddle::Function::STDCALL"]>.

@mrkn mrkn requested review from hsbt and removed request for hsbt October 29, 2018 01:55
This commit fixes the following failure:

```
  1) Failure:
Fiddle::TestImport#test_no_message_with_debug [/Users/mrkn/src/github.com/ruby/fiddle/test/fiddle/test_import.rb:152]:

1. [2/2] Assertion for "stderr"
   | <[]> expected but was
   | <["Exception `NameError' at /Users/mrkn/.rbenv/versions/2.5.1/lib/ruby/2.5.0/fiddle/import.rb:157 - uninitialized constant Fiddle::Function::STDCALL"]>.
```
@nobu
Copy link
Member

nobu commented Nov 2, 2018

Seems the bundled fiddle is called.
Don't mind it, my mistake.

@nobu
Copy link
Member

nobu commented Nov 12, 2018

I suspect that this failure is caused by a bug in bundler; bundler/setup.rb enables bundler even if rubygems is disabled.

@mrkn
Copy link
Member Author

mrkn commented May 27, 2019

@nobu I just remove -rbundler/setup from RUBYOPT instead of using Bundler.with_clean_env.

@mrkn
Copy link
Member Author

mrkn commented May 27, 2019

Oh...

@mrkn mrkn requested a review from kou May 27, 2019 08:32
@kou
Copy link
Member

kou commented May 27, 2019

How about disabling all RUBYOPT and RUBYLIB in this test? It's simpler and more stable.

This works even if a developer has the following ugly RUBYOPT:

$ cat /tmp/warn.rb
$, = "XXX"
$ RUBYOPT=-r/tmp/warn.rb rake
diff --git a/test/fiddle/test_import.rb b/test/fiddle/test_import.rb
index 99294ea..db58a82 100644
--- a/test/fiddle/test_import.rb
+++ b/test/fiddle/test_import.rb
@@ -149,7 +149,21 @@ module Fiddle
     end
 
     def test_no_message_with_debug
-      assert_in_out_err(%w[--debug --disable=gems -rfiddle/import], 'p Fiddle::Importer', ['Fiddle::Importer'])
+      # disable all Ruby environment variables
+      orig_RUBYOPT, ENV['RUBYOPT'] = ENV['RUBYOPT'], nil
+      orig_RUBYLIB, ENV['RUBYLIB'] = ENV['RUBYLIB'], nil
+      libdir = File.expand_path('../../../lib', __FILE__)
+      assert_in_out_err([
+                          "--debug",
+                          "--disable=gems",
+                          "-I#{libdir}",
+                          "-rfiddle/import",
+                        ],
+                        'p Fiddle::Importer',
+                        ['Fiddle::Importer'])
+    ensure
+      ENV['RUBYLIB'] = orig_RUBYLIB
+      ENV['RUBYOPT'] = orig_RUBYOPT
     end
   end
 end if defined?(Fiddle)

@mrkn
Copy link
Member Author

mrkn commented May 27, 2019

@kou thanks for your advice.
I pushed the new commit that follows your patch.

Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1

@kou kou merged commit 13133dd into ruby:master May 27, 2019
@nobu
Copy link
Member

nobu commented May 27, 2019

The first element of args to assert_in_out_err can be a Hash as environment variables.

@kou
Copy link
Member

kou commented May 27, 2019

!!!
I've changed to use it.

nobu pushed a commit to nobu/fiddle that referenced this pull request Apr 29, 2020
This reverts commit 5ebb0d50f6560b35bc03deb79341a115c5f782ee.
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.

3 participants