ruby / spec

The Ruby Spec Suite aka ruby/spec
MIT License
600 stars 389 forks source link

logname may not produce a login name #898

Open headius opened 3 years ago

headius commented 3 years ago

Running the specs on JRuby on Github Actions, we see the following message and failing spec:

...
logname: no login name
...
1)
Etc.getlogin returns the name associated with the current login activity FAILED
Expected "runner" == ""
to be truthy but was false
/home/runner/work/jruby/jruby/spec/ruby/library/etc/getlogin_spec.rb:22:in `block in <main>'
...

This can occur when logname is run without a controlling terminal. I'm unsure whether this GHA env is not setting up a tty, or if there's an issue launching the command in JRuby that prevents it inheriting the parent terminal, but it seems like id would be a more reliable command to use:

diff --git a/spec/ruby/library/etc/getlogin_spec.rb b/spec/ruby/library/etc/getlogin_spec.rb
index 7a4fd79ae2..f0dde84ccb 100644
--- a/spec/ruby/library/etc/getlogin_spec.rb
+++ b/spec/ruby/library/etc/getlogin_spec.rb
@@ -18,11 +18,13 @@ describe "Etc.getlogin" do
         else
           # Etc.getlogin returns the same result of logname(2)
           # if it returns non NULL
-          if system("which logname", out: File::NULL, err: File::NULL)
+          if system("which id", out: File::NULL, err: File::NULL)
+            Etc.getlogin.should == `id -un`.chomp
+          elsif system("which logname", out: File::NULL, err: File::NULL)
+            # fallback to `logname` command since `id` is not available
             Etc.getlogin.should == `logname`.chomp
           else
-            # fallback to `id` command since `logname` is not available
-            Etc.getlogin.should == `id -un`.chomp
+            Etc.getlogin.should == ENV['LOGNAME']
           end
         end
       else

However I think we are also stacking too many conditions here. I'm unsure of the "best" way to get the current login, but clearly logname has issues that make it undesirable.

FWIW some forums suggest logname -t which will ensure a tty is created, but this flag is not present on BSD-likes.

enebo commented 3 years ago

On a related question. MRI runs rubyspec in GHA right? Is there some option we can provide to run the job with a pty/tty?

eregon commented 3 years ago

ruby/spec's CI is on GHA (https://github.com/ruby/spec/blob/master/.github/workflows/ci.yml), so it seems the current spec works fine on CRuby on GHA Is it on ubuntu-latest?

If id works better than logname then it sounds fine to only try id. In that case, please make a PR and let's see if the CI works with it.

eregon commented 3 years ago

Maybe it's an issue of the JVM/JRuby not inheriting the controlling terminal for subprocesses?

eregon commented 3 years ago

Since the code was using which logname, maybe for ruby/spec and ruby/ruby that command is not available but for JRuby's GHA setup it's installed and hence it'd only fail there?

The comment above does seem to indicate logname is preferable.