Hi Pascal and Christian, Your patch looks good at first look, only a few comments about the code inline.
We are trying to standardize the results output format across our
testing components in order to build up-on a Test reporting tool, so let
us review the XML format.
Benjamin do you have a wiki page link with test result format information?
Cheers,
alimon
On 01/19/2017 07:33 AM, Pascal Bach wrote:
> From: Christian Schuler <schuler.christ...@siemens.com>
>
> add a feature that allows the user to generate an XUnit xml-file.
> This file shows if the ptests run correctly or not. e.g. bash, zlib.
> It doesn't list the underlaying tests of the package.
> The feature is activated by console input -x <filepath>.
> Unit-tests of the feature are included.
>
> Signed-off-by: Christian Schuler <schuler.christ...@siemens.com>
> Signed-off-by: Pascal Bach <pascal.b...@siemens.com>
> ---
> README.md | 1 +
> main.c | 15 +++++++++++----
> tests/utils.c | 46 +++++++++++++++++++++++++++++++++++++++++++--
> utils.c | 60
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> utils.h | 7 ++++++-
> 5 files changed, 121 insertions(+), 8 deletions(-)
>
> diff --git a/README.md b/README.md
> index ed7a589..fedab04 100644
> --- a/README.md
> +++ b/README.md
> @@ -14,6 +14,7 @@ Now the ptest-runner2 support the next features:
> - List available ptests.
> - Specify the timeout for avoid blocking indefinetly.
> - Only run certain ptests.
> +- XML-ouput
>
> Proposed features:
>
> diff --git a/main.c b/main.c
> index 31bf3b5..8dfdad6 100644
> --- a/main.c
> +++ b/main.c
> @@ -41,7 +41,7 @@
> static inline void
> print_usage(FILE *stream, char *progname)
> {
> - fprintf(stream, "Usage: %s [-d directory] [-l list] [-t timeout] "
> + fprintf(stream, "Usage: %s [-d directory] [-l list] [-t timeout] [-x
> xml-filename]"
> "[-h] [ptest1 ptest2 ...]\n", progname);
> }
>
> @@ -50,6 +50,7 @@ static struct {
> int list;
> int timeout;
> char **ptests;
> + char * xml_filename;
> } opts;
>
> int
> @@ -70,8 +71,9 @@ main(int argc, char *argv[])
> opts.list = 0;
> opts.timeout = DEFAULT_TIMEOUT;
> opts.ptests = NULL;
> + opts.xml_filename = NULL;
>
> - while ((opt = getopt(argc, argv, "d:lt:h")) != -1) {
> + while ((opt = getopt(argc, argv, "d:ltx:h")) != -1) {
> switch (opt) {
> case 'd':
> free(opts.directory);
> @@ -88,6 +90,11 @@ main(int argc, char *argv[])
> print_usage(stdout, argv[0]);
> exit(0);
> break;
> + case 'x':
> + free(opts.xml_filename);
> + opts.xml_filename = strdup(optarg);
> + CHECK_ALLOCATION(opts.xml_filename, 1, 1);
> + break;
> default:
> print_usage(stdout, argv[0]);
> exit(1);
> @@ -131,9 +138,9 @@ main(int argc, char *argv[])
> run = filter_ptests(head, opts.ptests, ptest_num);
> CHECK_ALLOCATION(run, ptest_num, 1);
> ptest_list_free_all(head);
> - }
> + }
>
> - rc = run_ptests(run, opts.timeout, argv[0], stdout, stderr);
> + rc = run_ptests(run, opts.timeout, argv[0], stdout, stderr,
> opts.xml_filename);
Will be a good idea modify the run_ptest signature to pass the whole
options to it.
I mean:
run_ptests(run, opts, argv[0], stdout, stderr)
If you could do a first patch with this change and then add your
functionality realted to xml_filename will be good.
>
> ptest_list_free_all(run);
>
> diff --git a/tests/utils.c b/tests/utils.c
> index 1332798..dd869e9 100644
> --- a/tests/utils.c
> +++ b/tests/utils.c
> @@ -173,7 +173,7 @@ START_TEST(test_run_ptests)
> ptest_list_remove(head, "hang", 1);
> ptest_list_remove(head, "fail", 1);
>
> - rc = run_ptests(head, timeout, "test_run_ptests", fp_stdout, fp_stderr);
> + rc = run_ptests(head, timeout, "test_run_ptests", fp_stdout, fp_stderr,
> NULL);
> ck_assert(rc == 0);
> ptest_list_free_all(head);
>
> @@ -234,6 +234,46 @@ START_TEST(test_run_fail_ptest)
> ptest_list_free_all(head);
> END_TEST
>
> +START_TEST(test_xml_pass)
> + char ref[] = "<?xml version='1.0' encoding='UTF-8'?>\n<testsuite
> name='ptests' tests='2'>\n"
> + "<testcase name='.'>\n<nodes expected='/' result='/'>\n"
> + "<nodes expected='0' result='0'>\n"
> + "</nodes>\n"
> + "</nodes>\n"
> + "</testcase>\n"
> + "<testcase name='.'>\n"
> + "<nodes expected='/' result='/'>\n"
> + "<nodes expected='0' result='1'>\n"
> + "<error type='Timeout/Unexpected error'>\n"
> + "A Timeout has occured or more tests than expected
> failed.\n"
> + "</error>\n"
> + "</nodes>\n"
> + "</nodes>\n"
> + "</testcase>\n"
> + "</testsuite>";
> + char buffer[strlen(ref)];
> +
> + FILE *fp;
> + fp = xml_create(2, "./test_xml");
> + xml_add_case(fp, 0,".");
> + xml_add_case(fp, 1,".");
> + xml_finish(fp);
> +
> + fp = fopen("./test_xml", "a+");
> + fseek(fp, 0, SEEK_SET);
> +
> + fread(buffer, strlen(ref), 1, fp);
> + ck_assert(strncmp(ref, buffer, strlen(ref)) == 0);
> + fclose(fp);
> + unlink("./test_xml");
> +
> +END_TEST
> +
> +START_TEST(test_xml_fail)
> + ck_assert(xml_create(2, "./") == NULL);
> +
> +END_TEST
> +
> Suite *
> utils_suite()
> {
> @@ -249,6 +289,8 @@ utils_suite()
> tcase_add_test(tc_core, test_run_ptests);
> tcase_add_test(tc_core, test_run_timeout_ptest);
> tcase_add_test(tc_core, test_run_fail_ptest);
> + tcase_add_test(tc_core, test_xml_pass);
> + tcase_add_test(tc_core, test_xml_fail);
>
> suite_add_tcase(s, tc_core);
>
> @@ -276,7 +318,7 @@ test_ptest_expected_failure(struct ptest_list *head,
> const int timeout, char *pr
> ck_assert(ptest_list_length(filtered) == 1);
>
> h_analizer(
> - run_ptests(filtered, timeout, progname, fp_stdout,
> fp_stderr),
> + run_ptests(filtered, timeout, progname, fp_stdout,
> fp_stderr, NULL),
> fp_stdout, fp_stderr
> );
>
> diff --git a/utils.c b/utils.c
> index 77427e0..430fb7a 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -303,9 +303,10 @@ wait_child(const char *ptest_dir, const char *run_ptest,
> pid_t pid,
>
> int
> run_ptests(struct ptest_list *head, int timeout, const char *progname,
> - FILE *fp, FILE *fp_stderr)
> + FILE *fp, FILE *fp_stderr, char *xml_filename)
> {
> int rc = 0;
> + XML *xh;
>
> struct ptest_list *p;
> char stime[GET_STIME_BUF_SIZE];
> @@ -314,6 +315,12 @@ run_ptests(struct ptest_list *head, int timeout, const
> char *progname,
> int pipefd_stdout[2];
> int pipefd_stderr[2];
>
> + if(xml_filename) {
> + xh = xml_create(ptest_list_length(head), xml_filename);
> + if(!xh)
> + exit(EXIT_FAILURE);
> + }
> +
> do
> {
> if ((rc = pipe2(pipefd_stdout, O_NONBLOCK)) == -1)
> @@ -355,6 +362,9 @@ run_ptests(struct ptest_list *head, int timeout, const
> char *progname,
> fprintf(fp, "ERROR: Exit status is
> %d\n", status);
> rc += 1;
> }
> + if (xml_filename)
> + xml_add_case(xh, status, ptest_dir);
> +
> fprintf(fp, "END: %s\n", ptest_dir);
> fprintf(fp, "%s\n", get_stime(stime,
> GET_STIME_BUF_SIZE));
> }
> @@ -368,5 +378,53 @@ run_ptests(struct ptest_list *head, int timeout, const
> char *progname,
> if (rc == -1)
> fprintf(fp_stderr, "run_ptests fails: %s", strerror(errno));
>
> + if (xml_filename)
> + xml_finish(xh);
> +
> return rc;
> }
> +
> +FILE *
> +xml_create(int test_count, char *xml_filename)
> +{
> + FILE *xh;
> +
> + if ((xh = fopen(xml_filename, "w"))) {
> + fprintf(xh, "<?xml version='1.0' encoding='UTF-8'?>\n");
> + fprintf(xh, "<testsuite name='ptests' tests='%d'>\n",
> test_count);
> + }
> + else {
> + fprintf(stderr, "File could not be created. Make sure the path
> is correct and you have the right permissions.");
> + return NULL;
> + }
> +
> + return xh;
> +}
> +
> +void
> +xml_add_case(FILE *xh, int status, const char *ptest_dir)
> +{
> + fprintf(xh, "<testcase name='%s'>\n", ptest_dir);
> + fprintf(xh, "<nodes expected='/' result='/'>\n");
> +
> + if(status == 0){
> + fprintf(xh, "<nodes expected='0' result='0'>\n");
> + fprintf(xh, "</nodes>\n");
> + }
> + else{
> + fprintf(xh, "<nodes expected='0' result='%d'>\n", status);
> + fprintf(xh, "<error type='Timeout/Unexpected error'>\n");
> + fprintf(xh, "A Timeout has occured or more tests than expected
> failed.\n");
> + fprintf(xh, "</error>\n");
> + fprintf(xh, "</nodes>\n");
> + }
> + fprintf(xh, "</nodes>\n");
> + fprintf(xh, "</testcase>\n");
> +}
> +
> +void
> +xml_finish(FILE *xh)
> +{
> + fprintf(xh, "</testsuite>");
> + fclose(xh);
> +}
> diff --git a/utils.h b/utils.h
> index 4dc23ef..99f6100 100644
> --- a/utils.h
> +++ b/utils.h
> @@ -30,10 +30,15 @@
> #define CHECK_ALLOCATION(p, size, exit_on_null) \
> check_allocation1(p, size, __FILE__, __LINE__, exit_on_null)
>
> +typedef FILE XML;
> +
> extern void check_allocation1(void *, size_t, char *, int, int);
> extern struct ptest_list *get_available_ptests(const char *);
> extern int print_ptests(struct ptest_list *, FILE *);
> extern struct ptest_list *filter_ptests(struct ptest_list *, char **, int);
> -extern int run_ptests(struct ptest_list *, int, const char *progname, FILE
> *, FILE *);
> +extern int run_ptests(struct ptest_list *, int, const char *progname, FILE
> *, FILE *, char *xml_filename);
> +extern XML * xml_create(int test_count, char *filename);
> +extern void xml_add_case(XML *, int status, const char *ptest_dir);
> +extern void xml_finish(XML *);
I'm trying to follow BSD code-style so please remove variable names from
prototypes. Also i make a mistake with progname that was before on the
run_ptests prototype.
Also i don't wrote about code style into README, i will put a section
into it, sorry for that.
Best regards,
alimon
>
> #endif
>
signature.asc
Description: OpenPGP digital signature
-- _______________________________________________ yocto mailing list yocto@yoctoproject.org https://lists.yoctoproject.org/listinfo/yocto
