diff mbox series

[v2] icu: use automake ptest output format

Message ID 20251031064504.461920-1-jiaying.song.cn@windriver.com
State New
Headers show
Series [v2] icu: use automake ptest output format | expand

Commit Message

Song, Jiaying (CN) Oct. 31, 2025, 6:45 a.m. UTC
From: Jiaying Song <jiaying.song.cn@windriver.com>

Make ICU ptest output compatible with Automake format.

Signed-off-by: Jiaying Song <jiaying.song.cn@windriver.com>
---
 ...utput-compatible-with-Automake-forma.patch | 132 ++++++++++++++++++
 meta/recipes-support/icu/icu_77-1.bb          |   1 +
 2 files changed, 133 insertions(+)
 create mode 100644 meta/recipes-support/icu/icu/0001-Make-ICU-ptest-output-compatible-with-Automake-forma.patch

Comments

Ross Burton Oct. 31, 2025, 2:41 p.m. UTC | #1
On 31 Oct 2025, at 06:45, Song, Jiaying (CN) via lists.openembedded.org <Jiaying.Song.CN=windriver.com@lists.openembedded.org> wrote:
> 
> +Upstream-Status: Pending

This isn’t not pending, as I doubt there’s any intention to get upstream to change the output of their test harness to suit us.

Please set it to "Inappropriate [OE-specific]”.

Ross
Alexander Kanavin Oct. 31, 2025, 4:30 p.m. UTC | #2
On Fri, 31 Oct 2025 at 07:45, <jiaying.song.cn@windriver.com> wrote:
> +- Adjust indentation and timing output formatting

This needs to be separately justified. Do we really need those extra
tweaks, or can the patch be limited to just adding PASS and FAIL
prints? Are those extra tweaks suitable for upstream submission?

Note that the longer the patch, the higher the likelihood that it will
fail to rebase on version updates, and require tedious manual rework
against upstream changes. Patches that stay in oe-core forever should
be as short as possible.

Alex
Song, Jiaying (CN) Nov. 3, 2025, 7:43 a.m. UTC | #3
Hi Alex, Ross,

Thanks for the feedback.
I’ve sent v3 to address your comments.

Best regards,
Jiaying

-----Original Message-----
From: Alexander Kanavin <alex.kanavin@gmail.com> 
Sent: Saturday, November 1, 2025 12:30 AM
To: Song, Jiaying (CN) <Jiaying.Song.CN@windriver.com>
Cc: openembedded-core@lists.openembedded.org
Subject: Re: [PATCH v2] icu: use automake ptest output format

CAUTION: This email comes from a non Wind River email account!
Do not click links or open attachments unless you recognize the sender and know the content is safe.

On Fri, 31 Oct 2025 at 07:45, <jiaying.song.cn@windriver.com> wrote:
> +- Adjust indentation and timing output formatting

This needs to be separately justified. Do we really need those extra tweaks, or can the patch be limited to just adding PASS and FAIL prints? Are those extra tweaks suitable for upstream submission?

Note that the longer the patch, the higher the likelihood that it will fail to rebase on version updates, and require tedious manual rework against upstream changes. Patches that stay in oe-core forever should be as short as possible.

Alex
diff mbox series

Patch

diff --git a/meta/recipes-support/icu/icu/0001-Make-ICU-ptest-output-compatible-with-Automake-forma.patch b/meta/recipes-support/icu/icu/0001-Make-ICU-ptest-output-compatible-with-Automake-forma.patch
new file mode 100644
index 0000000000..7ced43f723
--- /dev/null
+++ b/meta/recipes-support/icu/icu/0001-Make-ICU-ptest-output-compatible-with-Automake-forma.patch
@@ -0,0 +1,132 @@ 
+From 47e77a544076270eea4c7a814b6d435d1b74d5fa Mon Sep 17 00:00:00 2001
+From: Jiaying Song <jiaying.song.cn@windriver.com>
+Date: Thu, 30 Oct 2025 17:26:39 +0800
+Subject: [PATCH] Make ICU ptest output compatible with Automake format
+
+- Change test result format to PASS/FAIL for Automake compatibility
+- Adjust indentation and timing output formatting
+
+Upstream-Status: Pending
+
+Signed-off-by: Jiaying Song <jiaying.song.cn@windriver.com>
+---
+ test/intltest/intltest.cpp | 24 ++++++++++--------------
+ tools/ctestfw/ctest.c      | 14 +++++++-------
+ 2 files changed, 17 insertions(+), 21 deletions(-)
+
+diff --git a/test/intltest/intltest.cpp b/test/intltest/intltest.cpp
+index 33829b0..aa6c3ec 100644
+--- a/test/intltest/intltest.cpp
++++ b/test/intltest/intltest.cpp
+@@ -785,7 +785,7 @@ UBool IntlTest::runTestLoop( char* testname, char* par, char *baseName )
+     IntlTest* saveTest = gTest;
+     gTest = this;
+     do {
+-        this->runIndexedTest( index, false, name, par );
++        this->runIndexedTest(index, false, name, par );
+         if (strcmp(name,"skip") == 0) {
+             run_this_test = false;
+         } else {
+@@ -801,8 +801,9 @@ UBool IntlTest::runTestLoop( char* testname, char* par, char *baseName )
+             lastErrorCount = errorCount;
+             execCount++;
+             char msg[256];
+-            snprintf(msg, sizeof(msg), "%s {", name);
++            snprintf(msg, sizeof(msg), "Start the test %s ", name);
+             LL_message(UnicodeString(msg), true);
++            LL_indentlevel += 3;
+             UDate timeStart = uprv_getRawUTCtime();
+             strcpy(saveBaseLoc,name);
+             strcat(saveBaseLoc,"/");
+@@ -813,13 +814,9 @@ UBool IntlTest::runTestLoop( char* testname, char* par, char *baseName )
+ 
+             UDate timeStop = uprv_getRawUTCtime();
+             rval = true; // at least one test has been called
+-            char secs[256];
+-            if(!no_time) {
+-              snprintf(secs, sizeof(secs), "%f", (timeStop-timeStart)/1000.0);
+-            } else {
+-              secs[0]=0;
+-            }
+-            
++            char secs[256]= {0};
++            if(!no_time)
++               snprintf(secs, sizeof(secs), "%f", (timeStop-timeStart)/1000.0);
+ 
+             strcpy(saveBaseLoc,name);
+ 
+@@ -828,13 +825,14 @@ UBool IntlTest::runTestLoop( char* testname, char* par, char *baseName )
+             
+ 
+             saveBaseLoc[0]=0; /* reset path */
+-            
++           
++            LL_indentlevel -= 3; 
+             if (lastErrorCount == errorCount) {
+-                snprintf( msg, sizeof(msg),  "   } OK:   %s ", name );
++                snprintf(msg, sizeof(msg), "PASS: %s ", name);
+                 if(!no_time) str_timeDelta(msg+strlen(msg),timeStop-timeStart);
+                 lastTestFailed = false;
+             }else{
+-                snprintf(msg, sizeof(msg), "   } ERRORS (%li) in %s", static_cast<long>(errorCount - lastErrorCount), name);
++                snprintf(msg, sizeof(msg), "FAIL: %s (errors: %li)", name, static_cast<long>(errorCount - lastErrorCount));
+                 if(!no_time) str_timeDelta(msg+strlen(msg),timeStop-timeStart);
+ 
+                 for(int i=0;i<LL_indentlevel;i++) {
+@@ -844,7 +842,6 @@ UBool IntlTest::runTestLoop( char* testname, char* par, char *baseName )
+                 errorList += "\n";
+                 lastTestFailed = true;
+             }
+-            LL_indentlevel -= 3;
+             if (lastTestFailed) {
+                 LL_message({}, true);
+             }
+@@ -852,7 +849,6 @@ UBool IntlTest::runTestLoop( char* testname, char* par, char *baseName )
+             if (lastTestFailed) {
+                 LL_message({}, true);
+             }
+-            LL_indentlevel += 3;
+         }
+         index++;
+     }while(name);
+diff --git a/tools/ctestfw/ctest.c b/tools/ctestfw/ctest.c
+index 7634526..94abe05 100644
+--- a/tools/ctestfw/ctest.c
++++ b/tools/ctestfw/ctest.c
+@@ -359,16 +359,14 @@ static void iterateTestsWithLevel ( const TestNode* root,
+         strcat(pathToFunction, separatorString);
+     }
+     strcat(pathToFunction, nodeList[i]->name); /* including 'root' */
+-
+-    /* print test name and space. */
+-    INDENT_LEVEL = depth-1;
+-    if(root->name[0]) {
++    
++    if(root->test == NULL && root->name[0]) { 
+     	log_testinfo_i("%s ", root->name);
+-    } else {
++    } else if(root->test == NULL) {
+     	log_testinfo_i("(%s) ", ARGV_0);
++    } else {
++    	ON_LINE = true;
+     }
+-    ON_LINE = true;  /* we are still on the line with the test name */
+-
+ 
+     if ( (mode == RUNTESTS) &&
+ 		(root->test != NULL))  /* if root is a leaf node, run it */
+@@ -413,9 +411,11 @@ static void iterateTestsWithLevel ( const TestNode* root,
+         ctest_xml_testcase(pathToFunction, pathToFunction, timeSeconds, (myERROR_COUNT!=ERROR_COUNT)?"error":NULL);
+ 
+         if (myERROR_COUNT != ERROR_COUNT) {
++          log_testinfo_i("FAIL: %s", root->name);
+           log_testinfo_i("} ---[%d ERRORS in %s] ", ERROR_COUNT - myERROR_COUNT, pathToFunction);
+           strcpy(ERROR_LOG[ERRONEOUS_FUNCTION_COUNT++], pathToFunction);
+         } else {
++          log_testinfo_i("PASS: %s", root->name);
+           if(!ON_LINE) { /* had some output */
+             int spaces = FLAG_INDENT-(depth-1);
+             log_testinfo_i("} %*s[OK] ", spaces, "---");
+-- 
+2.34.1
+
diff --git a/meta/recipes-support/icu/icu_77-1.bb b/meta/recipes-support/icu/icu_77-1.bb
index fa25b3e7c8..8efdc83ce1 100644
--- a/meta/recipes-support/icu/icu_77-1.bb
+++ b/meta/recipes-support/icu/icu_77-1.bb
@@ -122,6 +122,7 @@  SRC_URI = "${BASE_SRC_URI};name=code \
            file://0001-ICU-23120-Mask-UnicodeStringTest-TestLargeMemory-on-.patch \
            file://0001-test-Add-support-ptest.patch \
            file://run-ptest \
+           file://0001-Make-ICU-ptest-output-compatible-with-Automake-forma.patch \
            "
 
 SRC_URI:append:class-target = "\