socketry / io-event

MIT License
66 stars 15 forks source link

Skip test on BSD systems. #62

Closed Math2 closed 1 year ago

Math2 commented 1 year ago

They usually have bidirectional pipes, which makes the test fail.

Types of Changes

Contribution

ioquatix commented 1 year ago

Instead of skipping the test, what about shutdown(WR) on it? i.e. IO#close_write.

Math2 commented 1 year ago

Instead of skipping the test, what about shutdown(WR) on it? i.e. IO#close_write.

Nah, shutdown is only for sockets on BSD. Pipes don't support it at all. But now that you mention it, closing the read end of the pipe makes the write end unwritable and it makes the test work.

Math2 commented 1 year ago

Oops actually scratch that, I made a mistake. If you close the "output" and then try to write on the "input", you'll get EPIPE (instead of EBADF on Linux).

So yeah that's something that could be tested instead of just skipping the test.

Math2 commented 1 year ago

What do you think?

I just tested, on Linux you always get EBADF even if you close the output beforehand. So I don't think this test can be made to be the same on both Linux and BSD.

diff --git i/test/io/event/selector/buffered_io.rb w/test/io/event/selector/buffered_io.rb
index c6b0ac5..5119235 100644
--- i/test/io/event/selector/buffered_io.rb
+++ w/test/io/event/selector/buffered_io.rb
@@ -39,11 +39,14 @@ BufferedIO = Sus::Shared("buffered io") do

        it "can't write to the read end of a pipe" do
            skip "Windows is bonkers" if RUBY_PLATFORM =~ /mswin|mingw|cygwin/
+           bsd = RUBY_PLATFORM =~ /bsd/
+           
+           output.close if bsd # BSD traditionally has bidirectional pipes

            writer = Fiber.new do
                buffer = IO::Buffer.new(64)
                result = selector.io_write(Fiber.current, input, buffer, 64)
-               expect(result).to be == -Errno::EBADF::Errno
+               expect(result).to be == -(bsd ? Errno::EPIPE : Errno::EBADF)::Errno
            end

            writer.transfer
ioquatix commented 1 year ago

This looks okay to me. Actually, the test is just that it's an error, we probably don't care much which error it is. It's invalid behaviour - it's more a test that errors are reported correctly.