[Tarantool-patches] [PATCH v3 0/2] Add option to update file with reference output

[Tarantool-patches] [PATCH v3 0/2] Add option to update file with reference output

[sergeyb]

From: Sergey Bronnikov <sergeyb@tarantool.org>

GH issue: https://github.com/tarantool/test-run/issues/194
GH branch: https://github.com/tarantool/test-run/tree/ligurio/gh-4654-update-ref-output

Sergey Bronnikov (2):
  Add flake8 config file to increase line minimal length
  Add option to update file with reference output

 .flake8        |  2 ++
 lib/options.py |  8 ++++++++
 lib/test.py    | 22 +++++++++++++++++-----
 3 files changed, 27 insertions(+), 5 deletions(-)
 create mode 100644 .flake8

-- 
2.23.0

[Tarantool-patches] [PATCH v3 1/2] Add flake8 config file to increase line minimal length

[sergeyb]

From: Sergey Bronnikov <sergeyb@tarantool.org>

---
 .flake8 | 2 ++
 1 file changed, 2 insertions(+)
 create mode 100644 .flake8

diff --git a/.flake8 b/.flake8
new file mode 100644
index 0000000..7da1f96
--- /dev/null
+++ b/.flake8
@@ -0,0 +1,2 @@
+[flake8]
+max-line-length = 100
-- 
2.23.0

[Tarantool-patches] [PATCH v3 2/2] Add option to update file with reference output

[sergeyb]

From: Sergey Bronnikov <sergeyb@tarantool.org>

New option covers two cases:
- in case of test failure test-run.py create a file .reject with
actual test output and one need to move .reject file to .result manually
when test has a valid behaviour. With option --update-result test-run.py
will do it automatically.
- in case of abcense reference result file test-run.py forced to create
such file in a source directory and set test status "new". This patch
changes behaviour. With option --update-result test status is "new"
and result file is created, without option test status is "fail" and
result file is not created.

Fixes https://github.com/tarantool/tarantool/issues/4654
Fixes https://github.com/tarantool/tarantool/issues/4258
Closes #194
---
 lib/options.py |  8 ++++++++
 lib/test.py    | 22 +++++++++++++++++-----
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/lib/options.py b/lib/options.py
index 8bacb4a..47bbc0f 100644
--- a/lib/options.py
+++ b/lib/options.py
@@ -201,6 +201,14 @@ class Options:
                 help="""Run the server under 'luacov'.
                 Default: false.""")
 
+        parser.add_argument(
+                "--update-result",
+                dest="update_result",
+                action="store_true",
+                default=False,
+                help="""Update or create file with reference output (.result).
+                Default: false.""")
+
         # XXX: We can use parser.parse_intermixed_args() on
         # Python 3.7 to understand commands like
         # ./test-run.py foo --exclude bar baz
diff --git a/lib/test.py b/lib/test.py
index 8bc1feb..be57224 100644
--- a/lib/test.py
+++ b/lib/test.py
@@ -152,7 +152,7 @@ class Test(object):
             it to stdout.
 
             Returns short status of the test as a string: 'skip', 'pass',
-            'new', or 'fail'. There is also one possible value for
+            'new', 'updated' or 'fail'. There is also one possible value for
             short_status, 'disabled', but it returned in the caller,
             TestSuite.run_test().
         """
@@ -232,12 +232,18 @@ class Test(object):
             color_stdout("[ pass ]\n", schema='test_pass')
             if os.path.exists(self.tmp_result):
                 os.remove(self.tmp_result)
-        elif (self.is_executed_ok and not
-              self.is_equal_result and not
-              os.path.isfile(self.result)) and not is_tap:
+        elif (self.is_executed_ok and
+              not self.is_equal_result and
+              not os.path.isfile(self.result) and not is_tap):
             shutil.copy(self.tmp_result, self.result)
             short_status = 'new'
             color_stdout("[ new ]\n", schema='test_new')
+        elif (self.is_executed_ok and
+              not self.is_equal_result and
+              lib.Options().args.update_result):
+            shutil.copy(self.tmp_result, self.result)
+            short_status = 'updated'
+            color_stdout("[ updated ]\n", schema='test_new')
         else:
             has_result = os.path.exists(self.tmp_result)
             if has_result:
@@ -321,10 +327,16 @@ class Test(object):
         try:
             tap.parse(content)
         except ValueError as e:
-            color_stdout('\nTAP13 parse failed: %s\n' % str(e),
+            color_stdout('\nTAP13 parse failed (%s).\n' % str(e),
                          schema='error')
+            color_stdout('\nNo result file (%s) found.\n' % self.result,
+                         schema='error')
+            if not lib.Options().args.update_result:
+                msg = 'Run the test with --update-result option to write the new result file.\n'
+                color_stdout(msg, schema='error')
             self.is_crash_reported = True
             return False, False
+
         is_ok = True
         for test_case in tap.tests:
             if test_case.result == 'ok':
-- 
2.23.0

Re: [Tarantool-patches] [PATCH v3 2/2] Add option to update file with reference output

[Alexander Turenko]

Made several changes, see below.

Pushed the patchset to test-run's master.

Updated the submodule in tarantool and pushed the update to master, 2.4,
2.3 and 1.10 branches.

WBR, Alexander Turenko.

> -        elif (self.is_executed_ok and not
> -              self.is_equal_result and not
> -              os.path.isfile(self.result)) and not is_tap:
> +        elif (self.is_executed_ok and
> +              not self.is_equal_result and
> +              not os.path.isfile(self.result) and not is_tap):
>              shutil.copy(self.tmp_result, self.result)
>              short_status = 'new'
>              color_stdout("[ new ]\n", schema='test_new')

We should write new result files only under --update-result. Added the check
for the option:

 |                  os.remove(self.tmp_result)
 |          elif (self.is_executed_ok and
 |                not self.is_equal_result and
 | -              not os.path.isfile(self.result) and not is_tap):
 | +              not os.path.isfile(self.result) and
 | +              not is_tap and
 | +              lib.Options().args.update_result):
 |              shutil.copy(self.tmp_result, self.result)
 |              short_status = 'new'
 |              color_stdout("[ new ]\n", schema='test_new')

> +        elif (self.is_executed_ok and
> +              not self.is_equal_result and
> +              lib.Options().args.update_result):
> +            shutil.copy(self.tmp_result, self.result)
> +            short_status = 'updated'
> +            color_stdout("[ updated ]\n", schema='test_new')

It will write a new result file for a failed TAP13 test under the
option. I guess it is not the expected behaviour in most of cases. Say,
I changed something in the net.box implementation and want to update
result files for all related tests in a batch (`./test-run.py
--update-result net.box`), but it should not write a result file (and
report successful testing) if some TAP13 test fails. Added the check:

 |              color_stdout("[ new ]\n", schema='test_new')
 |          elif (self.is_executed_ok and
 |                not self.is_equal_result and
 | +              os.path.isfile(self.result) and
 | +              not is_tap and
 |                lib.Options().args.update_result):
 |              shutil.copy(self.tmp_result, self.result)
 |              short_status = 'updated'

Re: [Tarantool-patches] [PATCH v3 2/2] Add option to update file with reference output

[Sergey Bronnikov]

On 17:36 Tue 09 Jun , Alexander Turenko wrote:
> Made several changes, see below.

thanks a lot!

> Pushed the patchset to test-run's master.
> 
> Updated the submodule in tarantool and pushed the update to master, 2.4,
> 2.3 and 1.10 branches.
> 
> WBR, Alexander Turenko.