Bug #14194
closed
--enable-frozen-string-literal ruby runner.rb cgi
Description
Assuming --enable-frozen-string-literal, ruby runner.rb cgi
Running without patch:
1) Failure:
CGIMultipartTest#test_cgi_multipart_without_tempfile [E:/GitHub/ruby/test/cgi/test_cgi_multipart.rb:353]:
1. [2/2] Assertion for "stderr"
| <[]> expected but was
| <["-:20:in `gsub!': can't modify frozen String, created at -:5 (FrozenError)",
| "\tfrom -:20:in `<main>'"]>.
Finished tests in 3.042000s, 143.3268 tests/s, 333.3333 assertions/s.
436 tests, 1014 assertions, 1 failures, 0 errors, 0 skips
With patch:
Finished tests in 2.620800s, 166.3614 tests/s, 386.9048 assertions/s.
436 tests, 1014 assertions, 0 failures, 0 errors, 0 skips
ruby -v: ruby 2.5.0dev (2017-12-17 trunk 61304) [x64-mingw32]
Files
Updated by jeremyevans0 (Jeremy Evans) almost 5 years ago
- Status changed from Open to Feedback
Now that it has been decided that Ruby 3 will not use frozen string literals by default, I think we should not assume that code that breaks with --enable-frozen-string-literal is buggy, just as we should not assume that code that breaks with --disable-gems is buggy. Do you still consider this a bug that we should fix?
Updated by MSP-Greg (Greg L) almost 5 years ago
Sorry for the tardy response, I had hoped to run a CI build with the flag set, but busy with other things...
I was unaware that the decision had been made re Ruby 3. If you have any links to discussions, etc, I'd be interested.
I have seen repos (Nokogiri comes to mind) testing with the flag set, so whatever parts of Ruby they're using are working.
In a perfect world, and given that I mostly recall seeing postive comments about using frozen strings, I think Ruby should pass CI with it.
I'll run a build with it and see how many tests/specs fail. This isn't a perfect world...
Thanks, Greg
Updated by nobu (Nobuyoshi Nakada) almost 5 years ago
It doesn’t seem harmful, and that test should be independent from the condition.
Updated by nobu (Nobuyoshi Nakada) almost 5 years ago
sorry, should be independent
Updated by Eregon (Benoit Daloze) almost 5 years ago
nobu (Nobuyoshi Nakada) wrote:
It doesn’t seem harmful, and tests should be independent from the condition.
I'm unsure, it forces existing code to add # frozen_string_literal: false, which seems not valuable if anyway the default is not going to change.
E.g., I don't think it's a valuable change to add # frozen_string_literal: false at the top of every ruby/spec file for now (I'm rather against it).
--enable-frozen-string-literal can break existing code, so I think we should document that in the man page/--help, if we don't remove the flag.
Updated by MSP-Greg (Greg L) almost 5 years ago
- File test_all_err.log test_all_err.log added
- File test_mspec_err.log test_mspec_err.log added
FYI,
Just ran ruby-loco with frozen string. Both test-all & spec are run parallel, and both crashed.
test-all failures/errors before crash:
ruby 2.7.0dev (2019-06-22T14:38:07Z master f738eeabc2) [x64-mingw32]
CRASHED?
3 CGIMultipartTest#test_cgi_multipart_without_tempfile = 0.06 s = F
/ruby/test/cgi/test_cgi_multipart.rb:353
1. [2/2] Assertion for "stderr"
| <[]> expected but was
| <["-:20:in `gsub!': can't modify frozen String: \"--foobar1234\\\\nContent-Disposition: form-data: name=\\\\\"name1\\\\\"\\\\n\\\\nvalue1\\\\n--foobar1234\\\\nContent-Disposition: form-data: name=\\\\\"file1\\\\\"; filename=\\\\\"file1.html\\\\\"\\\\nContent-Type: text/html\\\\n\\\\n<html>\\\\n<body><p>Hello</p></body>\\\\n</html>\\\\n\\\\n--foobar1234--\\\\n\" (FrozenError)",
| "\tfrom -:20:in `<main>'"]>.
2 TestObjSpace#test_dump_all = 0.04 s = F
/ruby/test/objspace/test_objspace.rb:382
Expected /"bytesize":11, "value":"TEST STRING", "encoding":"UTF-8", "file":"-", "line":4, "method":"dump_my_heap_please", "generation":/ to match "".
4 TestObjSpace#test_reachable_objects_from = 0.14 s = F
/ruby/test/objspace/test_objspace.rb:120
<[Array, "a", "a", "a"]> expected but was
<[Array, "a"]>.
0 Test_String_Fstring#test_singleton_class = 0.00 s = E
/ruby/test/-ext-/string/test_fstring.rb:61
can't modify frozen String: "_206aw_cd7_464323"
I can't look at this today, but I attached both STDERR logs which show the output from the crashes.
Thanks, Greg
Updated by MSP-Greg (Greg L) almost 5 years ago
- Status changed from Feedback to Closed
Applied in changeset git|2ad7a7f801c07f18150116b819530840eeefebf8.
Get rid of error with frozen string literal
[Bug #14194]