Cannot run full test suite against JS app

[letmaik]

As suggested by @jumaffre the full e2e test suite should run against the JS logging app. I tried to do that with:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 9398d5f93..27b38ef41 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -554,6 +554,8 @@ if(BUILD_TESTS)
     ADDITIONAL_ARGS
       --oe-binary
       ${OE_BINDIR}
+      --js-app-bundle
+                    ${CMAKE_SOURCE_DIR}/samples/apps/logging/js
       --ledger-recovery-timeout
       20
       --test-duration
diff --git a/tests/e2e_suite.py b/tests/e2e_suite.py
index e5bd70da4..151178957 100644
--- a/tests/e2e_suite.py
+++ b/tests/e2e_suite.py
@@ -193,7 +193,7 @@ if __name__ == "__main__":
         )
 
     args = infra.e2e_args.cli_args(add)
-    args.package = "liblogging"
+    args.package = "libjs_generic"
     args.nodes = infra.e2e_args.max_nodes(args, f=0)

But I get an error:

42: Traceback (most recent call last):
42:   File ".../CCF/tests/e2e_suite.py", line 200, in <module>
42:     run(args)
42:   File ".../CCF/tests/e2e_suite.py", line 151, in run
42:     network.stop_all_nodes(verbose_verification=False)
42:   File ".../CCF/tests/infra/network.py", line 523, in stop_all_nodes
42:     self.txs.verify(self, log_capture=log_capture)
42:   File ".../CCF/tests/infra/logging_app.py", line 155, in verify
42:     self._verify_tx(
42:   File ".../CCF/tests/infra/logging_app.py", line 214, in _verify_tx
42:     ccf.commit.wait_for_commit(
42:   File ".../CCF/python/ccf/commit.py", line 29, in wait_for_commit
42:     raise ValueError(f"{view}.{seqno} is not a valid transaction ID")
42: ValueError: None.None is not a valid transaction ID
1/1 Test #42: full_test_suite ..................***Failed   45.86 sec

[achamayou]

Should in what sense? The logging cpp app contains several endpoints that do not exist in the JS app, and aren't trivial to replicate there, for example the custom auth. That not all tests applied to one magically pass against the other is hardly a bug.

I'm in favour of extending test coverage, this is a good extension. But it's not the only way to resolve @jumaffre's valid concern about some tests that write to built-in tables not being covered by the test suite.

[jumaffre]

I believe that running the full test suite against the JS app is important as it'd be the only way to check, for example, that the endpoints table is successfully restored after a recovery. I'm surprised that the full test suite doesn't actually work since the e2e_logging test already run with the JS app.

@letmaik: Do you know which test fails in the suite and why?

[jumaffre]

As discussed with @achamayou, we definitely don't want to duplicate the full test suite as it's by far our most expensive test, nor only run the JS version as C++ apps are used in production today. Instead, we should add JS-specific test "steps" to the suite, which would perform a code upgrade on the fly to test those. This way, we would have used all built-in tables in the suite, and used them on either side of the recovery (using the randomised suite in the Daily CI).

[achamayou]

This isn't something we're going to address, efforts will be focused on ramping up testing on the external executor and bringing that up to par instead.