[PATCH v1] tests: getcwd.test

[Dmitry V. Levin]

On Tue, Mar 15, 2016 at 05:06:48PM +0530, Jay Joshi wrote:
> >From 5e543e3a0ae730b1833c8a4d5d9a5bf28273256d Mon Sep 17 00:00:00 2001
> From: JayRJoshi <jay.r.joshi100 at gmail.com>
> Date: Tue, 15 Mar 2016 16:41:39 +0530
> Subject: [PATCH] Test for getcwd (getcwd.c, getcwd.expected, getcwd.test)
>  added

This commit message doesn't follow our guidelines.
It might be a good idea to run "git log -- tests" and see how commit
messages for tests usually look like.

> --- /dev/null
> +++ b/tests/getcwd.c
> @@ -0,0 +1,36 @@
> +#include "tests.h"
> +
> +#include <sys/stat.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <errno.h>
> +
> +
> +int
> +main(void)
> +{
> +  char cur_dir[32];
> +
> +  if (mkdir("/tmp/sample_dir", S_IRWXU) == -1)
> +    perror_msg_and_skip("mkdir");

Why do you want to use /tmp in the first place?
Please avoid /tmp if possible, it's unreliable.

> +  if (chdir("/tmp/sample_dir") == -1)
> +    perror_msg_and_skip("chdir");
> +
> +  if (getcwd(cur_dir, sizeof(cur_dir)) != NULL) {
> +    printf("getcwd(\"/tmp/sample_dir\", 32) = 16\n");

Now that you use /tmp, imagine that /tmp is a symlink to another
directory.  In that case getcwd would return something different from your
expectations.

> +  } else {
> +    perror_msg_and_fail("getcwd");
> +  }
> +
> +  if (rmdir(cur_dir) == -1)
> +    perror_msg_and_skip("rmdir");
> +
> +  if (getcwd(cur_dir, sizeof(cur_dir)) != NULL) {
> +    printf("getcwd(\"/tmp/sample_dir\", 32) = 16\n");
> +  } else {
> +    printf("getcwd(\"0x0123456789ab, 32) = -1 ENOENT (%m)\n");

Why this magic number?

> --- /dev/null
> +++ b/tests/getcwd.expected
> @@ -0,0 +1,2 @@
> +getcwd\(\"/tmp/sample_dir\", 32\) += 16
> +getcwd\(0x[0-9a-f]+, 32\) += -1 ENOENT \(No such file or directory\)

Looks like you've got confused.  If you have getcwd.expected, then
getcwd.c doesn't have to print anything.  If your getcwd.c prints expected
output, then you don't need getcwd.expected.  In other words, you need
only one of two types of expected output, not both.

> diff --git a/tests/getcwd.test b/tests/getcwd.test
> new file mode 100755
> index 0000000..03fabb1
> --- /dev/null
> +++ b/tests/getcwd.test
> @@ -0,0 +1,9 @@
> +#!/bin/sh
> +
> +. "${srcdir=.}/init.sh"
> +
> +run_prog > /dev/null
> +run_strace -egetcwd $args
> +match_grep

Here your match_grep uses getcwd.expected, so the output produced by
getcwd.c is ignored.

I generally prefer match_diff to match_grep because the former is exact
comparison and therefore the check is more robust.


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20160317/1d7a5cd9/attachment.bin>