Command line test runner











up vote
2
down vote

favorite












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] = '';
const char *expected_result = args[delimiter_index + 1];
if (delimiter_index == 0 || expected_result == NULL)
{
fprintf(stderr, "Line <%s> is malformedn", 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 pipen");
return false;
}

pid_t pid = fork();
if (pid == -1)
{
fprintf(stderr, "Unable to fork processn");
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: %sn",
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 outputn");
return false;
}

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

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: %sn", expected_result);
fprintf(stderr, "Actual: %sn", 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 readingn", 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;
}









share|improve this question


























    up vote
    2
    down vote

    favorite












    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] = '';
    const char *expected_result = args[delimiter_index + 1];
    if (delimiter_index == 0 || expected_result == NULL)
    {
    fprintf(stderr, "Line <%s> is malformedn", 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 pipen");
    return false;
    }

    pid_t pid = fork();
    if (pid == -1)
    {
    fprintf(stderr, "Unable to fork processn");
    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: %sn",
    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 outputn");
    return false;
    }

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

    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: %sn", expected_result);
    fprintf(stderr, "Actual: %sn", 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 readingn", 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;
    }









    share|improve this question
























      up vote
      2
      down vote

      favorite









      up vote
      2
      down vote

      favorite











      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] = '';
      const char *expected_result = args[delimiter_index + 1];
      if (delimiter_index == 0 || expected_result == NULL)
      {
      fprintf(stderr, "Line <%s> is malformedn", 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 pipen");
      return false;
      }

      pid_t pid = fork();
      if (pid == -1)
      {
      fprintf(stderr, "Unable to fork processn");
      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: %sn",
      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 outputn");
      return false;
      }

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

      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: %sn", expected_result);
      fprintf(stderr, "Actual: %sn", 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 readingn", 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;
      }









      share|improve this 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] = '';
      const char *expected_result = args[delimiter_index + 1];
      if (delimiter_index == 0 || expected_result == NULL)
      {
      fprintf(stderr, "Line <%s> is malformedn", 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 pipen");
      return false;
      }

      pid_t pid = fork();
      if (pid == -1)
      {
      fprintf(stderr, "Unable to fork processn");
      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: %sn",
      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 outputn");
      return false;
      }

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

      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: %sn", expected_result);
      fprintf(stderr, "Actual: %sn", 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 readingn", 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;
      }






      c unix c99






      share|improve this question













      share|improve this question











      share|improve this question




      share|improve this question










      asked Dec 12 at 18:27









      User319

      734413




      734413






















          3 Answers
          3






          active

          oldest

          votes

















          up vote
          3
          down vote



          accepted












          • fprintf(stderr, "Unable to read program outputn"); 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.








          share|improve this answer




























            up vote
            2
            down vote













            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] = '';


            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: %sn", expected_result);
            fprintf(stderr, "Actual: %sn", actual_result);


            can be something like



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





            share|improve this answer




























              up vote
              1
              down vote













              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.






              share|improve this answer





















                Your Answer





                StackExchange.ifUsing("editor", function () {
                return StackExchange.using("mathjaxEditing", function () {
                StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
                StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
                });
                });
                }, "mathjax-editing");

                StackExchange.ifUsing("editor", function () {
                StackExchange.using("externalEditor", function () {
                StackExchange.using("snippets", function () {
                StackExchange.snippets.init();
                });
                });
                }, "code-snippets");

                StackExchange.ready(function() {
                var channelOptions = {
                tags: "".split(" "),
                id: "196"
                };
                initTagRenderer("".split(" "), "".split(" "), channelOptions);

                StackExchange.using("externalEditor", function() {
                // Have to fire editor after snippets, if snippets enabled
                if (StackExchange.settings.snippets.snippetsEnabled) {
                StackExchange.using("snippets", function() {
                createEditor();
                });
                }
                else {
                createEditor();
                }
                });

                function createEditor() {
                StackExchange.prepareEditor({
                heartbeatType: 'answer',
                autoActivateHeartbeat: false,
                convertImagesToLinks: false,
                noModals: true,
                showLowRepImageUploadWarning: true,
                reputationToPostImages: null,
                bindNavPrevention: true,
                postfix: "",
                imageUploader: {
                brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
                contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
                allowUrls: true
                },
                onDemand: true,
                discardSelector: ".discard-answer"
                ,immediatelyShowMarkdownHelp:true
                });


                }
                });














                draft saved

                draft discarded


















                StackExchange.ready(
                function () {
                StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f209552%2fcommand-line-test-runner%23new-answer', 'question_page');
                }
                );

                Post as a guest















                Required, but never shown

























                3 Answers
                3






                active

                oldest

                votes








                3 Answers
                3






                active

                oldest

                votes









                active

                oldest

                votes






                active

                oldest

                votes








                up vote
                3
                down vote



                accepted












                • fprintf(stderr, "Unable to read program outputn"); 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.








                share|improve this answer

























                  up vote
                  3
                  down vote



                  accepted












                  • fprintf(stderr, "Unable to read program outputn"); 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.








                  share|improve this answer























                    up vote
                    3
                    down vote



                    accepted







                    up vote
                    3
                    down vote



                    accepted








                    • fprintf(stderr, "Unable to read program outputn"); 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.








                    share|improve this answer














                    • fprintf(stderr, "Unable to read program outputn"); 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.









                    share|improve this answer












                    share|improve this answer



                    share|improve this answer










                    answered Dec 12 at 20:49









                    vnp

                    38.4k13096




                    38.4k13096
























                        up vote
                        2
                        down vote













                        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] = '';


                        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: %sn", expected_result);
                        fprintf(stderr, "Actual: %sn", actual_result);


                        can be something like



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





                        share|improve this answer

























                          up vote
                          2
                          down vote













                          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] = '';


                          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: %sn", expected_result);
                          fprintf(stderr, "Actual: %sn", actual_result);


                          can be something like



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





                          share|improve this answer























                            up vote
                            2
                            down vote










                            up vote
                            2
                            down vote









                            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] = '';


                            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: %sn", expected_result);
                            fprintf(stderr, "Actual: %sn", actual_result);


                            can be something like



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





                            share|improve this answer












                            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] = '';


                            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: %sn", expected_result);
                            fprintf(stderr, "Actual: %sn", actual_result);


                            can be something like



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






                            share|improve this answer












                            share|improve this answer



                            share|improve this answer










                            answered Dec 12 at 19:31









                            Reinderien

                            2,062616




                            2,062616






















                                up vote
                                1
                                down vote













                                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.






                                share|improve this answer

























                                  up vote
                                  1
                                  down vote













                                  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.






                                  share|improve this answer























                                    up vote
                                    1
                                    down vote










                                    up vote
                                    1
                                    down vote









                                    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.






                                    share|improve this answer












                                    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.







                                    share|improve this answer












                                    share|improve this answer



                                    share|improve this answer










                                    answered Dec 13 at 10:30









                                    Toby Speight

                                    23.3k639111




                                    23.3k639111






























                                        draft saved

                                        draft discarded




















































                                        Thanks for contributing an answer to Code Review Stack Exchange!


                                        • Please be sure to answer the question. Provide details and share your research!

                                        But avoid



                                        • Asking for help, clarification, or responding to other answers.

                                        • Making statements based on opinion; back them up with references or personal experience.


                                        Use MathJax to format equations. MathJax reference.


                                        To learn more, see our tips on writing great answers.





                                        Some of your past answers have not been well-received, and you're in danger of being blocked from answering.


                                        Please pay close attention to the following guidance:


                                        • Please be sure to answer the question. Provide details and share your research!

                                        But avoid



                                        • Asking for help, clarification, or responding to other answers.

                                        • Making statements based on opinion; back them up with references or personal experience.


                                        To learn more, see our tips on writing great answers.




                                        draft saved


                                        draft discarded














                                        StackExchange.ready(
                                        function () {
                                        StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f209552%2fcommand-line-test-runner%23new-answer', 'question_page');
                                        }
                                        );

                                        Post as a guest















                                        Required, but never shown





















































                                        Required, but never shown














                                        Required, but never shown












                                        Required, but never shown







                                        Required, but never shown

































                                        Required, but never shown














                                        Required, but never shown












                                        Required, but never shown







                                        Required, but never shown







                                        Popular posts from this blog

                                        Список кардиналов, возведённых папой римским Каликстом III

                                        Deduzione

                                        Mysql.sock missing - “Can't connect to local MySQL server through socket”