alot icon indicating copy to clipboard operation
alot copied to clipboard

test_no_spawn_no_stdin_attached tests the build environment

Open kpcyrd opened this issue 4 years ago • 7 comments

hello, just letting you know alot currently fails in the Arch Linux reproducible builds system.

Since the original build passed it seems the test heavily depends on the terminal attached to the build, which it shouldn't.

test_char_special (tests.utils.test_argparse.TestRequireFile) ... ok
test_dir (tests.utils.test_argparse.TestRequireFile) ... ok
test_doesnt_exist (tests.utils.test_argparse.TestRequireFile) ... ok
test_fifo (tests.utils.test_argparse.TestRequireFile) ... ok
test_file (tests.utils.test_argparse.TestRequireFile) ... ok
test_validates (tests.utils.test_argparse.TestValidatedStore) ... ok
test_empty_strings_are_converted_to_empty_lists (tests.utils.test_configobj.TestForceList) ... ok
test_strings_are_converted_to_single_item_lists (tests.utils.test_configobj.TestForceList) ... ok
test_hash_for_unicode_representation (tests.widgets.test_globals.TestTagWidget) ... ok
test_sort (tests.widgets.test_globals.TestTagWidget)
Test sorting. ... ok

======================================================================
FAIL: test_no_spawn_no_stdin_attached (tests.commands.test_global.TestExternalCommand)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/build/alot/src/alot-0.9.1/tests/utilities.py", line 189, in _actual
    return loop.run_until_complete(coro(*args, **kwargs))
  File "/usr/lib/python3.9/asyncio/base_events.py", line 642, in run_until_complete
    return future.result()
  File "/build/alot/src/alot-0.9.1/tests/commands/test_global.py", line 129, in test_no_spawn_no_stdin_attached
    ui.notify.assert_not_called()
  File "/usr/lib/python3.9/unittest/mock.py", line 868, in assert_not_called
    raise AssertionError(msg)
AssertionError: Expected 'notify' to not have been called. Called 1 times.
Calls: [call('editor has exited with error code 1 -- No stderr output', priority='error')].

----------------------------------------------------------------------
Ran 345 tests in 3.432s

FAILED (failures=1, expected failures=9)
Test failed: <unittest.runner.TextTestResult run=345 errors=0 failures=1>
error: Test failed: <unittest.runner.TextTestResult run=345 errors=0 failures=1>
==> ERROR: A failure occurred in check().
    Aborting...
[1m[34m  ->[0m[1m Delete snapshot for alot_3270817...[0m

kpcyrd avatar Jul 13 '21 19:07 kpcyrd

Thanks for reporting. Is this a broken test or alot's functionality malforming? Either way, a PR would be welcome. Cheers, P

pazz avatar Jul 15 '21 10:07 pazz

I think it's only the test, it only affects some build environments and the application itself is probably fine.

kpcyrd avatar Jul 15 '21 16:07 kpcyrd

The test code is this:

    @utilities.async_test
    async def test_no_spawn_no_stdin_attached(self):
        ui = utilities.make_ui()
        cmd = g_commands.ExternalCommand('test -t 0', refocus=False)
        await cmd.apply(ui)
        ui.notify.assert_not_called()

The ExternalCommand class is just mainly a wrapper around asyncio.create_subprocess_{exec,shell} and here we run test -t 0. In the next test (which succeeded) we run test -t 0 while sending some text to it from python and then expect the command to fail.

@kpcyrd I think "it seems the test heavily depends on the terminal attached to the build" is a little to strong, my conclusion would be "the test depends an any terminal being attached". And then my question is: Is there a terminal attached to the build? What kind of infrastructure are you using to run these tests? Do I assume correctly that test -t 0 in a shell would fail in that infrastructure?

And the question @pazz and @dcbaker is: This test was introduced in 5f96f0d , do we really need a terminal or can we test this some other way? What exactly are we trying to test?

lucc avatar Jul 15 '21 19:07 lucc

That set of tests is fixed in the next commit, which seems to be for #1137, so what's supposed to be tested is that the subprocess.Popen call is invoked with it's stdin parameter populated correctly.

dcbaker avatar Jul 15 '21 22:07 dcbaker

The same test was troubling me when running tests in github ci: https://github.com/pazz/alot/pull/1617/commits/7bf4dee4ea7288e0392aa6c9e2053062fcecffb5

lucc avatar Dec 08 '23 10:12 lucc

Sorry for not following up for 2 years, the infrastructure this is running on is https://reproducible.archlinux.org/, the chain of execution is rebuilderd -> archlinux-repro -> makepkg -> alot tests, archlinux-repro and makepkg just pass through stdin/stdout/stderr, but rebuilderd explicitly sets /dev/null for stdin and a pipe for stdout and stderr to capture the child processes output:

https://github.com/kpcyrd/rebuilderd/blob/0fda367320ef18b65dcf6890c168b67a9cff65df/worker/src/proc.rs#L122-L146

This is done so the website can show the build output for all the packages we build.

kpcyrd avatar Dec 08 '23 12:12 kpcyrd

We have a nix flake in this repository and nix also sets stdin to /dev/null in the build sandbox. I just verified this and inside the build sandbox of nix I get

lrwxrwxrwx 1 nobody nogroup 15 Dec  8 14:44 /dev/stderr -> /proc/self/fd/2
lrwxrwxrwx 1 nobody nogroup 15 Dec  8 14:44 /dev/stdin -> /proc/self/fd/0
lrwxrwxrwx 1 nobody nogroup 15 Dec  8 14:44 /dev/stdout -> /proc/self/fd/1
lrwx------ 1 nixbld nixbld  64 Dec  8 14:44 /proc/self/fd/0 -> /dev/null
lrwx------ 1 nixbld nixbld  64 Dec  8 14:44 /proc/self/fd/1 -> /dev/pts/2
lrwx------ 1 nixbld nixbld  64 Dec  8 14:44 /proc/self/fd/2 -> /dev/pts/2

So if the test succeeds in the nix sandbox it should also succeed in the arch sandbox.

lucc avatar Dec 08 '23 14:12 lucc