Add failing test for function cloning

[SamSaffron]

No description provided.

[SamSaffron]

@bnoordhuis I just tried latest on Discourse and it is failing due to this test:

  def test_function_cloning
    message = nil
    context = MiniRacer::Context.new
    context.attach("rails.logger.warn", proc { |msg| message = msg })
    context.eval <<~JS
               console = {
                 prefix: "[PrettyText] ",
                 log: function(...args){ rails.logger.info(console.prefix + args.join(" ")); },
               }
             JS

    context.eval("console.log('Hello', 'World!')")
    assert_equal "[PrettyText] Hello World!", message
  end

[bnoordhuis]

Typo? It says rails.logger.warn on one line and rails.logger.info on another.

[SamSaffron]

@bnoordhuis sorry about the typo fixed it, but test is still broken due to clone issue

[bnoordhuis]

I see what the problem is: the result of this statement is (edit: an object containing) the log function object. The test passes when changed to this:

Suggested change

	console = {
	log: function(...args){ log(args.join(" ")); },
	}
	console = {
	log: function(...args){ log(args.join(" ")); },
	}
	undefined

Let me think what a good solution is.

[bnoordhuis]

For completeness sake, another workaround is to make the property non-enumerable:

Suggested change

	console = {
	log: function(...args){ log(args.join(" ")); },
	}
	console = {}
	Object.defineProperty(console, "log", {value: function(...args){ log(args.join(" ")); }})

[SamSaffron]

Ideally we can just return something here , trouble of crashing on deserialozation boundary is that many eval and don’t care about result , even just saying “function” is fine

[bnoordhuis]

For posterity, I think there are 4 options:

1. do nothing and let the error bubble up (current situation)
2. detect the error and return something else, like an empty object or a marker
3. deep-copy the object and replace or drop function properties
4. ditto but only shallow-copy the object, i.e., fix up only one level

3 is O(n) time and space, 4 is only a partial fix, so I guess 2 is the best option.