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

Sprockets test_compile_erb_template_that_depends_on_env test fails #2810

Closed
eregon opened this issue Dec 19, 2022 · 6 comments
Closed

Sprockets test_compile_erb_template_that_depends_on_env test fails #2810

eregon opened this issue Dec 19, 2022 · 6 comments

Comments

@eregon
Copy link
Member

eregon commented Dec 19, 2022

These 2 tests fail, it's unclear why.
https://github.com/rails/sprockets/blob/1276b431e2e4c1099dae1b3ff76adc868c863ddd/test/test_erb_processor.rb#L31
https://github.com/rails/sprockets/blob/1276b431e2e4c1099dae1b3ff76adc868c863ddd/test/test_erb_processor.rb#L56

https://github.com/rails/sprockets/blob/1276b431e2e4c1099dae1b3ff76adc868c863ddd/lib/sprockets/erb_processor.rb#L30 is what should track ENV accesses.

$ cd sprockets
$ bundle exec rake test TEST=/home/eregon/code/sprockets/test/test_erb_processor.rb
  1) Failure:
TestERBProcessor#test_compile_erb_template_that_depends_on_env [/home/eregon/code/sprockets/test/test_erb_processor.rb:51]:
Expected: "env:ERB_ENV_TEST_VALUE"
  Actual: nil

  2) Failure:
TestERBProcessor#test_compile_erb_template_that_depends_on_empty_env [/home/eregon/code/sprockets/test/test_erb_processor.rb:75]:
Expected: "env:ERB_ENV_TEST_VALUE"
  Actual: nil
eregon added a commit to eregon/sprockets that referenced this issue Dec 19, 2022
* This should help catching concurrency issues much faster.
* Notably, the two issues of rails#772 reproduce by running the test suite on TruffleRuby.
* Skip tests which do not pass yet on TrufffleRuby.
  oracle/truffleruby#2810
@eregon
Copy link
Member Author

eregon commented Dec 20, 2022

It'd be good to also fix this skip: https://github.com/rails/sprockets/pull/771/files#diff-400bee1b426336ba5800f5dad93859b63a235c7214c598edc69bc4b0e1f1c9aaR55
i.e. Marshal.load("") raising ArgumentError and not TypeError.

@andrykonchin
Copy link
Member

A simplified example to reproduce the issue:

require 'erb'

C = 1

class A
end

a = A.new

klass = (class << a; self; end)
klass.const_set(:C, 2)

engine = ERB.new('<%= C %>')
puts engine.result(a.instance_eval('binding'))

Results:

ruby const2.rb
2
ruby -v
ruby 3.1.3p185 (2022-11-24 revision 1a6b16756e) [x86_64-darwin21]

ruby const2.rb
1
ruby -v
truffleruby 23.0.0-dev-0d398ebd, like ruby 3.1.3, Interpreted JVM [x86_64-darwin]

@andrykonchin
Copy link
Member

A bit simpler example (without binding and const_set calls):

C = 1

class A
  C = 2
end

a = A.new
puts a.instance_eval("C")

@eregon
Copy link
Member Author

eregon commented Dec 23, 2022

I see, so then we need to add the receiver of instance_eval first in constant lookup, i.e. a LexicalScope object

@eregon
Copy link
Member Author

eregon commented Jan 4, 2023

C = 1

class A
  C = 2
end

a = A.new
a.singleton_class.const_set(:C, 3)

puts a.instance_eval("C") # => 3

@andrykonchin
Copy link
Member

Fixed in f911355.

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

No branches or pull requests

2 participants