Command line test runner

Question

This is a simple program to run a command-line program with given parameters and verify that it writes the expected output to stdout or stderr. The specification file is passed as the first argument. It's formatted like this:

echo foo -> foo
man -> What manual page do you want?

with program and arguments to the left of -> and output on the right.

I don't often work so low-level, so I'm most interested in whether I'm managing resources properly.

#define _GNU_SOURCE

#include <errno.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>


#define MAX_ARGS 10
#define MAX_LINE 256


const char* parse_args(char **args, const char *line)
{
    size_t index = 0;
    size_t delimiter_index = 0;
    while (args[index] != NULL && index < MAX_ARGS - 1)
    {
        ++index;
        args[index] = strtok(NULL, " ");
        if (args[index] && strncmp(args[index], "->", 2) == 0)
        {
            delimiter_index = index;
            args[index+1] = strtok(NULL, "\n");
            break;
        }
    }

    args[delimiter_index] = '\0';
    const char *expected_result = args[delimiter_index + 1];
    if (delimiter_index == 0 || expected_result == NULL)
    {
        fprintf(stderr, "Line <%s> is malformed\n", line);
        return NULL;
    }

    if (strlen(expected_result) > MAX_LINE)
    {
        fprintf(stderr,
                "Invalid specification. Only results less than %d chars supported",
                MAX_LINE);
        return NULL;
    }

    return expected_result;
}

bool run_test(char *line)
{
    char *args[MAX_ARGS] = {};
    args[0] = strtok(line, " ");
    if (!args[0]) return true;

    const char* expected_result = parse_args(args, line);
    if (!expected_result)
    {
        return false;
    }

    int stdout_pipe[2];
    if (pipe(stdout_pipe) != 0)
    {
        fprintf(stderr, "Unable to open pipe\n");
        return false;
    }

    pid_t pid = fork();
    if (pid == -1)
    {
        fprintf(stderr, "Unable to fork process\n");
        return false;
    }

    if (pid == 0)
    {
        // Redirect streams so caller can read stdout
        dup2(stdout_pipe[1], STDOUT_FILENO);
        dup2(stdout_pipe[1], STDERR_FILENO);
        close(stdout_pipe[0]);
        execvp(args[0], args);

        // exec* only ever returns if it fails
        fprintf(stderr, "Failed to execute test program: %s\n",
                strerror(errno));
        return false;
    }

    close(stdout_pipe[1]);

    char actual_result[MAX_LINE] = {};
    ssize_t bytes_read = read(stdout_pipe[0], actual_result, MAX_LINE);
    if (bytes_read == -1)
    {
        fprintf(stderr, "Unable to read program output\n");
        return false;
    }

    // Strip newline
    if (actual_result[bytes_read - 1] == '\n')
    {
        actual_result[bytes_read - 1] = '\0';
    }

    if (strcmp(actual_result, expected_result) != 0)
    {
        fprintf(stderr, "Test Failed: ");
        for (int arg = 0; arg < MAX_ARGS; ++ arg)
        {
            if (!args[arg]) break;
            fprintf(stderr, "%s ", args[arg]);
        }
        fprintf(stderr, "\n");
        fprintf(stderr, "Expected: %s\n", expected_result);
        fprintf(stderr, "Actual: %s\n", actual_result);
        return false;
    }

    return true;
}

int main(int argc, char *argv[])
{
    if (argc != 2)
    {
        fprintf(stderr, "Usage: %s <test_specification>\n", argv[0]);
        return 1;
    }

    const char *spec_pathname = argv[1];
    FILE* spec = fopen(spec_pathname, "r");
    if (!spec)
    {
        fprintf(stderr, "Cannot open <%s> for reading\n", spec_pathname);
        return 1;
    }

    int return_code = 0;

    char *line = NULL;
    size_t line_length = 0;
    while (getline(&line, &line_length, spec) != -1)
    {
        if (!run_test(line))
        {
            return_code = 1;
        }
    }

    free(line);
    return return_code;
}

Answer 1

• fprintf(stderr, "Unable to read program output\n"); loses important information: exactly why the program output couldn't be read. Prefer perror.

  Ditto for other fprintf(stderr, ....).

• The parent doesn't wait for children. You may produce plenty of zombies.

• The parent doesn't close(stdout_pipe[0]); when done with a child. You may run out of file descriptors.

• The spec file format seems naive:

  • There is no way to deal with a multi-line output.
  • It mixes child's stdout and stderr. If the child produces both, their order is unpredictable.
  • It doesn't specify the child's return status.

Answer 2

while (args[index] != NULL && index < MAX_ARGS - 1)

I suggest that you refactor this loop a little bit; here's one option:

for (size_t index = 1; index < MAX_ARGS && args[index]; index++) { // ...

This:

args[delimiter_index] = '\0';

if it works without warning, that's only by accident. You're confusing pointer assignment with character assignment. It should actually be

args[delimiter_index] = NULL;

In all likelihood, both parse_args and run_test should be static because no one else is importing them.

For code like

if (pipe(stdout_pipe) != 0)

You should strongly consider calling perror to get a human-readable error string.

You can combine these fprintf calls and still have the strings on separate lines:

    fprintf(stderr, "\n");
    fprintf(stderr, "Expected: %s\n", expected_result);
    fprintf(stderr, "Actual: %s\n", actual_result);

can be something like

    fprintf(stderr, "\n"
                    "Expected: %s\n"
                    "Actual: %s\n", expected_result, actual_result);

Answer 3

Conformance

A couple of violations of Standard C:

209552.c: In function ‘run_test’:
209552.c:52:28: warning: ISO C forbids empty initializer braces [-Wpedantic]
     char *args[MAX_ARGS] = {};
                            ^
209552.c:92:36: warning: ISO C forbids empty initializer braces [-Wpedantic]
     char actual_result[MAX_LINE] = {};
                                    ^

Another pedantic point, which you're probably aware of: this clearly targets POSIX systems, where we are guaranteed that argc is at least 1, and so argv[0] is always usable; however, when writing portable programs, code like this can be dangerous:

if (argc != 2)
{
    fprintf(stderr, "Usage: %s <test_specification>\n", argv[0]);
    return 1;
}

Input file format

There are some severe limitations to the input file format. It's impossible to specify commands or arguments that contain space characters, and it's impossible to specify output of more than one line.

Addressing the last point first, perhaps we should consider writing one file per test, with the command as first line, and then all subsequent lines being the expected output. Adapt the program to read loop over the command-line arguments, reading every file that's specified. It's easy for users to test lots of commands, using wildcards (e.g. run_tests *.test).

As for the first problem, we could consider using a shell to parse it instead of strtok() - just take the whole command and pass it to /bin/sh -c.

Perhaps we also want to check the exit status of the program under test - I think that's an important part of the program's interface.

Output format

For syntax errors in the file format, we could improve the error message by writing the file name and line number, rather than just the contents. This would then be consistent with error messages from compilers and other tools, which can be parsed (such as in Emacs, where goto-error will take the user directly to the problem line).

Parsing function

The parsing might be simpler if we first divide at -> (or newline, in my proposed input format), and then process the input and output sides separately. We really should emit a good error when the MAX_ARGS limit is violated (as we do for MAX_LINE). A worthwhile enhancement would be to eliminate these arbitrary limits.