vadimdemedes / ink-testing-library

Utilities for testing Ink apps
MIT License
109 stars 17 forks source link

Test if app has exited #5

Open mamachanko opened 5 years ago

mamachanko commented 5 years ago

I would like to test whether my app has quit. Ink's render returns waitUntilExit which works neatly. From what I can tell there's no tooling for this provided by the testing library. That is either on purpose or because there was no demand yet.

So I tried to experimentally expose the instance's waitUntilExit by the testing library's render. However, it doesn't resolve. When I manually call unmount though it does.

import * as React from 'react';
import {Box, Text} from 'ink';
import {render} from 'ink-testing-library';

const App = () => (
    <Box flexDirection="column">
        <Text>Hello.</Text>
        <Text>And Goodbye.</Text>
    </Box>
);

it('says hello and goodbye', () => {
    const {lastFrame} = render(<App/>);

    expect(lastFrame()).toMatch(/Hello\.\s*And Goodbye\.\s*/);

    // Has it quit though?
});

it('says hello, goodbye and quits when it\'s done - 1', () => {
    const {lastFrame, waitUntilExit} = render(<App/>);

    expect(lastFrame()).toMatch(/Hello\.\s*And Goodbye\.\s*/);

    // Doesn't resolve
    return waitUntilExit();
});

it('says hello, goodbye and quits when it\'s done - 2', () => {
    const {lastFrame, unmount, waitUntilExit} = render(<App/>);

    expect(lastFrame()).toMatch(/Hello\.\s*And Goodbye\.\s*/);

    unmount();

    // Resolves
    return waitUntilExit();
});

I haven't yet sufficiently understood how ink decides when to exit the process. Is it when there are no more listeners to stdin or when its reconciler figures that there's nothing new to render anymore?

Would be happy to work on this but might need some help.

vadimdemedes commented 5 years ago

Ink never exits your process, process can exit "by itself" if Ink app was unmounted and there's nothing running anymore. So that's actually an expected behavior for waitUntilExit() to not resolve until unmount() is called.

Since Ink doesn't exit the process, not sure it's useful to test that app is unmounted. Do you mind sharing the reason for this test?

mamachanko commented 5 years ago

I think it's an essential property of a terminal application to eventually and predictably exit. Unless it's a long-running process of course, in which case you expect it not to exit at all until killed or quit.

Something might be going wrong if the application exits unexpectedly or not at all. It would be great if one could test an ink app's termination behaviour.

ink already exposes waitUntilExit() for that purpose. That is the reason why I thought it would also "just work"™ by exposing it via ink-testing-library.

In the spirit of the testing library's guiding principles (and this tweet) I thought it would be nice to assume the user's point-of-view in a similar fashion. After all, it's ink-testing-library's name-sake. A user is not concerned with an app unmounting (they might not be concerned about what happens under the hood at all, neither should they), but they are concerned with an app exiting.

vadimdemedes commented 5 years ago

I think it would make sense to expose that if Ink actually terminated the process or handled how it exits. However, Ink doesn't manage the process. waitUntilExit() method is only useful for embedded Ink apps, where they're only a part of a larger terminal app with its own exit handling. See https://github.com/vadimdemedes/ink/issues/125.

For waitUntilExit() to work consistently across all cases and always resolve whenever app is unmounted or process is actually about to exit, Ink would need to use https://github.com/tapjs/signal-exit.

I think in the case of testing Ink apps, it would make more sense to expose something like isExited property, which seems like would've solved your case. What do you think?

mamachanko commented 5 years ago

Thanks for the explanation. Yes, that sounds like good idea! Do you think of isExited as a boolean or a function?

vadimdemedes commented 5 years ago

I think just as a boolean property.

mamachanko commented 5 years ago

One wouldn't be able to test if an app exits after a certain input / event etc., would one?

const {stdin, isExited} = render(<App/>);

expect(isExited).toBe(false);

stdin.write('pew pew')

expect(isExited).toBe(true);
vadimdemedes commented 5 years ago

I think you'd be able to do that, why not? You'd just have to wrap stdin.write() into act(), but there's an issue with it (see https://github.com/vadimdemedes/ink-testing-library/issues/3). Until then, you could simply wait a few hundred milliseconds and then perform an assertion for isExited === true.

mAAdhaTTah commented 5 years ago

I think you'd be able to do that, why not?

Because once you've destructured isExited, it won't get updated if it changes internally. It probably should be a function.