[PATCH] osmo-pcu[master]: bitcomp test: fix: only one hexdump per log; use printf

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Neels Hofmeyr gerrit-no-reply at lists.osmocom.org
Sun Mar 26 22:25:08 UTC 2017


Review at  https://gerrit.osmocom.org/2168

bitcomp test: fix: only one hexdump per log; use printf

The test wants to write multiline results, so it should use printf instead of
the logging system.

Split logs to only one hexdump per printf(). One cannot use osmo_hexdump twice
in one printf(); before this, one of the two hexdumps won, both dumps would
show as the same. Very bad for a regression test!

This uncovers a discrepancy between expected and produced results, proving that
the expected stderr output was not capable of uncovering test failures. The
test's check_result() function *has* always verified the decoded data, but only
up to the last decoded bit. Our expected data contains seemingly random bits
after the end of the decoded bits, but check_result() never compares those,
hence we don't catch that error. The extra bits should definitely be zero,
because the destination buffer is pre-initialized to zero -- fixed in a
subsequent patch.

This should cosmetically fix the build failure found in:
http://lists.osmocom.org/pipermail/osmocom-net-gprs/2017-March/000876.html
  [osmo-pcu 0.2.896-0a8f] testsuite: 4 failed
  from: Arnaud ZANETTI
  on: Fri Mar 24 09:53:53 UTC 2017
The real fix will follow.

Change-Id: I24fc32eb55baaf22f9c6fdda917bfb8395d02b1c
---
M tests/bitcomp/BitcompTest.cpp
M tests/bitcomp/BitcompTest.err
2 files changed, 17 insertions(+), 19 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/68/2168/1

diff --git a/tests/bitcomp/BitcompTest.cpp b/tests/bitcomp/BitcompTest.cpp
index 31aebd4..f35d6be 100644
--- a/tests/bitcomp/BitcompTest.cpp
+++ b/tests/bitcomp/BitcompTest.cpp
@@ -18,6 +18,8 @@
 #define MAX_CRBB_LEN 23
 #define MAX_URBB_LEN 40
 #define CEIL_DIV_8(x) (((x) + 7)/8)
+#define _LOG(fmt, args...) \
+	fprintf(stderr, fmt, ## args)
 
 void *tall_pcu_ctx;
 
@@ -157,8 +159,7 @@
 		dest.data = bits_data;
 		dest.data_len = sizeof(bits_data);
 		dest.cur_bit = 0;
-		LOGP(DRLCMACDL, LOGL_DEBUG,
-		     "\nTest:%d\n"
+		_LOG("\nTest:%d\n"
 		     "Tree based decoding:\n"
 		     "uncompressed data = %s\n"
 		     "len = %d\n",
@@ -169,8 +170,7 @@
 		rc = egprs_compress::decompress_crbb(test[itr].crbb_len,
 			test[itr].cc, test[itr].crbb_data, &dest);
 		if (rc < 0) {
-			LOGP(DRLCMACUL, LOGL_NOTICE,
-			     "\nFailed to decode CRBB: length %d, data %s",
+			_LOG("\nFailed to decode CRBB: length %d, data %s",
 			     test[itr].crbb_len,
 			     osmo_hexdump(test[itr].crbb_data,
 					  CEIL_DIV_8(test[itr].crbb_len)));
@@ -178,29 +178,27 @@
 		if (test[itr].verify) {
 			if (!result_matches(dest, test[itr].ucmp_data,
 					    test[itr].ucmp_len)) {
-				LOGP(DRLCMACDL, LOGL_DEBUG,
-				     "\nTree based decoding: Error\n"
+				_LOG("\nTree based decoding: Error\n"
 				     "expected data = %s\n"
-				     "expected len = %d\n"
-				     "decoded data = %s\n"
-				     "decoded len = %d\n",
+				     "expected len = %d\n",
 				     osmo_hexdump(test[itr].ucmp_data,
 						  CEIL_DIV_8(test[itr].ucmp_len)),
-				     test[itr].ucmp_len,
+				     test[itr].ucmp_len);
+				_LOG("decoded data = %s\n"
+				     "decoded len = %d\n",
 				     osmo_hexdump(dest.data,
 						  CEIL_DIV_8(dest.cur_bit)),
 				     dest.cur_bit);
 				OSMO_ASSERT(0);
 			}
 		}
-		LOGP(DRLCMACDL, LOGL_DEBUG,
-		     "\nexpected data = %s\n"
-		     "expected len = %d\n"
-		     "decoded data = %s\n"
-		     "decoded len = %d\n",
+		_LOG("\nexpected data = %s\n"
+		     "expected len = %d\n",
 		     osmo_hexdump(test[itr].ucmp_data,
 				  CEIL_DIV_8(test[itr].ucmp_len)),
-		     test[itr].ucmp_len,
+		     test[itr].ucmp_len);
+		_LOG("decoded data = %s\n"
+		     "decoded len = %d\n",
 		     osmo_hexdump(dest.data, CEIL_DIV_8(dest.cur_bit)),
 		     dest.cur_bit);
 	}
diff --git a/tests/bitcomp/BitcompTest.err b/tests/bitcomp/BitcompTest.err
index 7481d72..f769daa 100644
--- a/tests/bitcomp/BitcompTest.err
+++ b/tests/bitcomp/BitcompTest.err
@@ -13,7 +13,7 @@
 
 expected data = ff ff ff f8 00 00 01 ff ff ff f8 00 00 00 ff ff ff fe 00 00 3f ff ff ff db 
 expected len = 194
-decoded data = ff ff ff f8 00 00 01 ff ff ff f8 00 00 00 ff ff ff fe 00 00 3f ff ff ff db 
+decoded data = ff ff ff f8 00 00 01 ff ff ff f8 00 00 00 ff ff ff fe 00 00 3f ff ff ff c0 
 decoded len = 194
 
 Test:2
@@ -27,7 +27,7 @@
 
 expected data = ff ff ff ff ff ff c0 00 00 00 00 3f ff ff ff ff ff f8 00 00 00 00 03 
 expected len = 182
-decoded data = ff ff ff ff ff ff c0 00 00 00 00 3f ff ff ff ff ff f8 00 00 00 00 03 
+decoded data = ff ff ff ff ff ff c0 00 00 00 00 3f ff ff ff ff ff f8 00 00 00 00 00 
 decoded len = 182
 
 Test:3
@@ -106,7 +106,7 @@
 
 expected data = 
 expected len = 0
-decoded data = 
+decoded data = b0 00 00 
 decoded len = 19
 
 Test:8

-- 
To view, visit https://gerrit.osmocom.org/2168
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I24fc32eb55baaf22f9c6fdda917bfb8395d02b1c
Gerrit-PatchSet: 1
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>



More information about the gerrit-log mailing list