[PATCH v1] tests: getcwd.test

Thu Mar 17 07:53:17 UTC 2016

On Thu, Mar 17, 2016 at 7:49 AM, Dmitry V. Levin <ldv at altlinux.org> wrote:

> 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.
>

Actually, I wanted to avoid using regexp for directory path. Also, /tmp was
by default (and usually) is given all rwx permissions for all users.
Although, I do agree with your point.


> > +  } 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.
>
>
I wanted to cover test for error condition ENOENT, I couldn't think of any
way to check it using match_diff. Is it possible?
Updated patch is pasted below. I'm using regexp for directories. As, no
nested regexp is used, I don't think performance will be concern.

>From 27c04f259a56c2990511ab171ddbb283f0810b95 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] tests: add getcwd.test

---
 tests/.gitignore      |  1 +
 tests/Makefile.am     |  2 ++
 tests/getcwd.c        | 30 ++++++++++++++++++++++++++++++
 tests/getcwd.expected |  2 ++
 tests/getcwd.test     |  9 +++++++++
 5 files changed, 44 insertions(+)
 create mode 100644 tests/getcwd.c
 create mode 100644 tests/getcwd.expected
 create mode 100755 tests/getcwd.test

diff --git a/tests/.gitignore b/tests/.gitignore
index b89796a..52eb76d 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -41,6 +41,7 @@ fstat64
 fstatat64
 ftruncate
 ftruncate64
+getcwd
 getdents
 getdents64
 getrandom
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 59e7e78..81ad778 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -91,6 +91,7 @@ check_PROGRAMS = \
  fstatat64 \
  ftruncate \
  ftruncate64 \
+ getcwd \
  getdents \
  getdents64 \
  getrandom \
@@ -264,6 +265,7 @@ TESTS = \
  fstatat64.test \
  ftruncate.test \
  ftruncate64.test \
+ getcwd.test \
  getdents.test \
  getdents64.test \
  getrandom.test \
diff --git a/tests/getcwd.c b/tests/getcwd.c
new file mode 100644
index 0000000..561a4f1
--- /dev/null
+++ b/tests/getcwd.c
@@ -0,0 +1,30 @@
+#include "tests.h"
+
+#include <sys/stat.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <errno.h>
+
+#define sample_dir "getcwd.sample_dir"
+#define path_size 2048
+int
+main(void)
+{
+  char cur_dir[path_size];
+
+  if (mkdir(sample_dir, S_IRWXU) == -1)
+    perror_msg_and_skip("mkdir");
+
+  if (chdir(sample_dir) == -1)
+    perror_msg_and_skip("chdir");
+
+  if (getcwd(cur_dir, path_size) == NULL)
+    perror_msg_and_fail("getcwd");
+
+  if (rmdir(cur_dir) == -1)
+    perror_msg_and_skip("rmdir");
+
+  getcwd(cur_dir, path_size);
+
+  return 0;
+}
diff --git a/tests/getcwd.expected b/tests/getcwd.expected
new file mode 100644
index 0000000..301c8ac
--- /dev/null
+++ b/tests/getcwd.expected
@@ -0,0 +1,2 @@
+getcwd\(\"[^\0]+"\, 2048\) += [0-9]+
+getcwd\(0x[0-9a-f]+, 2048\) += -1 ENOENT \(No such file or directory\)
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
+
+exit 0
-- 
1.9.1
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20160317/40fb38b5/attachment.html>
