oracle / truffleruby

A high performance implementation of the Ruby programming language, built on GraalVM.
https://www.graalvm.org/ruby/
Other
3.02k stars 185 forks source link

Sprockets test_compile_erb_template_that_depends_on_env test fails #2810

Closed eregon closed 1 year ago

eregon commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

eregon commented 1 year ago
C = 1

class A
  C = 2
end

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

puts a.instance_eval("C") # => 3
andrykonchin commented 1 year ago

Fixed in f9113553823106072f9979b72ccaae9a7e372119.