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

Fix console_test to make pass after IRB's behavior change #51667

Merged
merged 1 commit into from May 6, 2024

Conversation

tompng
Copy link
Contributor

@tompng tompng commented Apr 26, 2024

Motivation / Background

In IRB's pull request ruby/irb#907, IRB's behavior with ENV['TERM']=dumb will change.
After the change, some test in railties/test/application/console_test.rb will fail like this.

Failure:
FullStackConsoleTest#test_console_respects_user_defined_prompt_mode [test/application/console_test.rb:124]:
">> 123" expected, but got:


=> 123
>> .
Expected "\r\n=> 123\r\n>> " to include ">> 123".

This pull request makes console_test pass.

Detail

write_prompt "123", "prompt> 123" will execute

@primary.puts("123")
assert_output("123", @primary)
assert_output("prompt> 123", @primary)
assert_output("> ", @primary)

Output from IRB will change like this

prompt> prompt> 1prompt> 12prompt> 123 (Reline <= 0.5.2) prompt> 112123 (Reline >= 0.5.3)
prompt> 123 (Reline Prints prompt and whole input again)
123 (IRB's --verbose option prints this)
=> 123
prompt> (Reline prints next prompt)

prompt> 123 (IRB prints "promt> ", PTY echos "123")
123 (IRB's --verbose option prints this)
=> 123
prompt> (IRB prints next prompt)

This pull request changes it to
write_prompt "123", prompt: "prompt> " which will execute below, and correctly matches in both cases.

@primary.puts("123")
assert_output("123", @primary)
assert_output("prompt> ", @primary)

Additional information

Adding -- --nomultiline --nosingleline will emulate IRB's behavior change

--- a/railties/test/application/console_test.rb
+++ b/railties/test/application/console_test.rb
@@ -128,7 +128,7 @@ def write_prompt(command, expected_output = nil, prompt: "> ")
   def spawn_console(options, wait_for_prompt: true, env: {})
     pid = Process.spawn(
       { "TERM" => "dumb" }.merge(env),
-      "#{app_path}/bin/rails console #{options}",
+      "#{app_path}/bin/rails console #{options} #{'--' unless options.include?(' -- ')} --nomultiline --nosingleline",
       in: @replica, out: @replica, err: @replica
     )

TERM=dumb is used in emacs shell. IRB/Reline used to print many redundant test and make it unusable.

% TERM=dumb irb
irb(main):001> 123
irb(main):001> 1irb(main):001> 12irb(main):001> 123=> 123
irb(main):002> 

The old test was matching to this redundant output part.

@rails-bot rails-bot bot added the railties label Apr 26, 2024
@yahonda
Copy link
Member

yahonda commented Apr 30, 2024

Let's revisit this pull request once the newer version if irb that includes ruby/irb#907

@st0012
Copy link
Contributor

st0012 commented May 5, 2024

@yahonda I've released IRB 1.13.1, which includes ruby/irb#907.

@tompng tompng force-pushed the console_test_with_stdin_input_method branch from 1ad7615 to 4b46041 Compare May 5, 2024 11:47
@yahonda yahonda merged commit 942e05b into rails:main May 6, 2024
3 of 4 checks passed
@yahonda
Copy link
Member

yahonda commented May 6, 2024

Thanks.

@tompng tompng deleted the console_test_with_stdin_input_method branch May 6, 2024 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants