From f559724d625ac4ccd03d203141562aadced534b2 Mon Sep 17 00:00:00 2001 From: Xavier Delaruelle Date: Sat, 15 Aug 2026 09:12:46 +0000 Subject: [PATCH 1/5] ts: cover invalid MODULES_COLOR env var override The color config init proc ignores the Modules-specific env var override when its value is not part of the valid value list for that option, falling back to whatever value was already determined (default or NO_COLOR/CLICOLOR derived). The existing MODULES_COLOR coverage only ever set valid values (never/auto/always), so that ignore branch was never exercised. Add a case setting MODULES_COLOR to an invalid value and checking color rendering matches the default configuration, confirming the override is ignored. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle --- testsuite/modules.00-init/140-color.exp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/testsuite/modules.00-init/140-color.exp b/testsuite/modules.00-init/140-color.exp index 64f48ad1d..e5f1d6b59 100644 --- a/testsuite/modules.00-init/140-color.exp +++ b/testsuite/modules.00-init/140-color.exp @@ -191,6 +191,15 @@ setenv_var MODULES_COLOR always color_test_case {} 2 $sgr unsetenv_var MODULES_COLOR +# an invalid Modules-specific color config env var override is ignored +setenv_var MODULES_COLOR foo +if {$install_color eq {y}} { + color_test_case {} 1 $sgr +} else { + color_test_case {} 0 $sgr +} +unsetenv_var MODULES_COLOR + skip_if_quick_mode # environment variable or/and command-line switch override From d56d4ae79dffe9894e1a6a961307631a139e3f70 Mon Sep 17 00:00:00 2001 From: Xavier Delaruelle Date: Sat, 15 Aug 2026 09:12:56 +0000 Subject: [PATCH 2/5] Remove unreachable intvallist guard in color config init The color config value internal-representation conversion was guarded by a check on intvallist length, but this proc is only ever registered as the init callback for the color option, always with the same non-empty intvallist. The guard can never evaluate false. Drop the guard, keeping its body unconditional. This changes nothing at runtime since the branch was unreachable, so no new test is needed. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle --- tcl/init.tcl.in | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tcl/init.tcl.in b/tcl/init.tcl.in index 2616b57d2..402f875c7 100644 --- a/tcl/init.tcl.in +++ b/tcl/init.tcl.in @@ -730,9 +730,7 @@ proc initConfColor {envvar value validvallist intvallist} { } # convert value to its internal representation - if {[llength $intvallist]} { - set value [lindex $intvallist [lsearch -exact $validvallist $value]] - } + set value [lindex $intvallist [lsearch -exact $validvallist $value]] # disable color mode if no terminal attached except if 'always' asked if {$value != 0 && (![getState is_stderr_tty] || $value == 2)} { From 8bf32aa75a0b0cbebf85885b1bd4e4a71a8e302d Mon Sep 17 00:00:00 2001 From: Xavier Delaruelle Date: Sat, 15 Aug 2026 09:13:09 +0000 Subject: [PATCH 3/5] ts: cover eval-wrapped topmost frame in error stack trace The error stack trace filtering proc special-cases a topmost frame whose command is 'eval', extracting the second word of that frame to compare it against the internal command filter list instead of the literal word 'eval' -- needed since modulefile content is evaluated through an 'eval [...]' wrapper around the proc that fetches it. The existing synthetic siteconfig case for this proc used an unquoted, hand-written frame that never matched this shape, leaving that branch untested. Add a second synthetic case whose topmost frame is a properly quoted 'eval [...]' wrapper, matching the real shape produced when evaluating modulefile content. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle --- testsuite/example/siteconfig.tcl-1 | 15 +++++++++++++++ testsuite/modules.00-init/120-siteconfig.exp | 10 ++++++++++ 2 files changed, 25 insertions(+) diff --git a/testsuite/example/siteconfig.tcl-1 b/testsuite/example/siteconfig.tcl-1 index 75a21d6ef..c3763986c 100644 --- a/testsuite/example/siteconfig.tcl-1 +++ b/testsuite/example/siteconfig.tcl-1 @@ -381,6 +381,21 @@ foo report [formatErrStackTrace $errmsg foo] } +# specific test to improve formatErrStackTrace proc coverage: the topmost +# stack frame is itself an 'eval [...]' wrapper (as modulefile content is +# evaluated via 'eval [getModuleContent $::ModulesCurrentModulefile]'), which +# requires extracting the inner command from the second word of that frame +if {[info exists env(TESTSUITE_ENABLE_SITECONFIG_FORMATERRSTACKTRACE_EVAL)]} { + set errmsg {error message + while executing +"eval [getModuleContent $::ModulesCurrentModulefile]" + invoked from within +"eval [getModuleContent $::ModulesCurrentModulefile]" + (file "foo")} + + report [formatErrStackTrace $errmsg foo] +} + # produce an unknown error in a modulecmd.tcl procedure during a modulefile evaluation if {[info exists env(TESTSUITE_ENABLE_SITECONFIG_UNKINTERRDURINGMODEVAL)]} { proc getModshareVarName {var} { diff --git a/testsuite/modules.00-init/120-siteconfig.exp b/testsuite/modules.00-init/120-siteconfig.exp index 59dfaf291..0004d1c7f 100644 --- a/testsuite/modules.00-init/120-siteconfig.exp +++ b/testsuite/modules.00-init/120-siteconfig.exp @@ -651,6 +651,16 @@ lappend ans $vers_reportre testouterr_cmd_re sh -V OK [join $ans \n] unsetenv_var TESTSUITE_ENABLE_SITECONFIG_FORMATERRSTACKTRACE +# specific test to improve formatErrStackTrace proc coverage: topmost stack +# frame is itself an 'eval [...]' wrapper +setenv_var TESTSUITE_ENABLE_SITECONFIG_FORMATERRSTACKTRACE_EVAL 1 +set ans [list] +lappend ans {} +lappend ans { \(file "foo"\)} +lappend ans $vers_reportre +testouterr_cmd_re sh -V OK [join $ans \n] +unsetenv_var TESTSUITE_ENABLE_SITECONFIG_FORMATERRSTACKTRACE_EVAL + # produce an unknown error in a modulecmd.tcl procedure during a modulefile evaluation setenv_var TESTSUITE_ENABLE_SITECONFIG_UNKINTERRDURINGMODEVAL 1 set ans [msg_moderr {invalid command name "foo"} {foo} $modpath/setenv/1.0 30 {} {} {} { (procedure "getModshareVarName" line 2) From 75321e2b0b4dfe081fd311f64890936a613585a1 Mon Sep 17 00:00:00 2001 From: Xavier Delaruelle Date: Sat, 15 Aug 2026 09:13:18 +0000 Subject: [PATCH 4/5] ts: cover comments-only modulespath init file The startup logic that reads the site-wide modulespath init file skips the module-use call entirely when every line is a comment (no path collected), a case distinct from the file being absent or containing actual paths -- both already covered. Add a case that temporarily swaps the installed modulespath file for a comments-only variant and checks a fresh autoinit adds no extra search path, restoring the original content afterward. Verified against a probe copy of the built interpreter that the skip branch is reached and the expected output produced (a local coverage run was inconclusive: other, pre-existing environment-dependent failures elsewhere in this same test file abort the build before the coverage markup step runs, on a checkout not fully installed to its configured prefix). Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle --- testsuite/modules.70-maint/120-autoinit.exp | 55 +++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/testsuite/modules.70-maint/120-autoinit.exp b/testsuite/modules.70-maint/120-autoinit.exp index 4834e22cb..566aea27a 100644 --- a/testsuite/modules.70-maint/120-autoinit.exp +++ b/testsuite/modules.70-maint/120-autoinit.exp @@ -2197,6 +2197,61 @@ if {$insmodspath && $modspath_exinstalled && $modspath eq "$install_etcdir/modul } +# test a modulespath file present but yielding no additional path entries +# (only comments) correctly adds no extra 'module use --append' and behaves +# just as if no modulespath file were installed at all +if {$insmodspath && $modspath_exinstalled && $modspath eq "$install_etcdir/modulespath"\ + && !$modrc_exinstalled && !$modrc1_exinstalled && !$initrc_exinstalled} { + # temporarily replace etc/modulespath with a comments-only variant + set fid [open $modspath r] + set origmodspathdata [read $fid] + close $fid + set fid [open $env(TESTSUITEDIR)/example/modulespath-empty r] + set emptymodspathdata [read $fid] + close $fid + set fid [open $modspath w] + puts -nonewline $fid $emptymodspathdata + close $fid + + set ans [list] + lappend ans [list [set "func_sh"]] + if {$install_versioning eq "y"} { + lappend ans [list set MODULE_VERSION_STACK "(.*)"] + } + lappend ans [list set MODULES_CMD $modulecmd_path] + if {$change_manpath} { + if {$extra_sysmanpath} { + lappend ans [list set __MODULES_SHARE_MANPATH :1] + } + lappend ans [list set MANPATH $updatedmanpath] + } + if {$change_path} { + lappend ans [list set PATH $updatedpath] + } + if {$install_setshellstartup eq {y}} { + lappend ans [list set ENV $install_initdir/profile.sh] + } + lappend ans [list unset __MODULES_AUTOINIT_INPROGRESS] + if {$install_setshellstartup eq {y}} { + lappend ans [list set BASH_ENV $install_initdir/bash] + } + lappend ans [list set MODULESHOME $moduleshome] + lappend ans [list set LOADEDMODULES ""] + lappend ans [list set __MODULES_LMINIT ""] + lappend ans [list set MODULEPATH ""] + if {$install_versioning eq "y"} { + lappend ans [list set MODULE_VERSION "(.*)"] + } + + testouterr_cmd_re sh autoinit $ans {} + + # restore original etc/modulespath content + set fid [open $modspath w] + puts -nonewline $fid $origmodspathdata + close $fid +} + + # ensure ml is set disabled as if modrc1 initrc were previously evaluated # since modulepath and modules are loaded, initrc will not be evaluated by autoinit if {$modrc1_exinstalled} { From e9082de7e95ea4480f7f1bf663007a15f7b1cf44 Mon Sep 17 00:00:00 2001 From: Xavier Delaruelle Date: Sat, 15 Aug 2026 11:18:24 +0200 Subject: [PATCH 5/5] gh: drop Codecov token, upload works without one for fork PRs Base repo secrets aren't exposed to pull_request runs triggered from forks, so CODECOV_TOKEN was always empty for fork PRs and the upload silently failed. Codecov can identify public repos automatically via GitHub's identity checks, so no token is needed and coverage now uploads for fork PRs too. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle --- .github/workflows/linux_tests.yaml | 30 ++++++++++++++++++------------ .github/workflows/macos_tests.yaml | 5 +++-- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/.github/workflows/linux_tests.yaml b/.github/workflows/linux_tests.yaml index 522849ee2..adf0625c9 100644 --- a/.github/workflows/linux_tests.yaml +++ b/.github/workflows/linux_tests.yaml @@ -67,9 +67,10 @@ jobs: - name: Uninstall Modules run: | make uninstall + # no token needed: public repo uses Codecov's tokenless upload via + # GitHub OIDC, which also works for pull requests coming from forks + # (base repo secrets are never exposed to fork-triggered PR runs) - uses: codecov/codecov-action@v7 - with: - token: ${{ secrets.CODECOV_TOKEN }} - uses: actions/upload-artifact@v7 if: failure() with: @@ -154,9 +155,10 @@ jobs: - name: Uninstall Modules run: | make uninstall + # no token needed: public repo uses Codecov's tokenless upload via + # GitHub OIDC, which also works for pull requests coming from forks + # (base repo secrets are never exposed to fork-triggered PR runs) - uses: codecov/codecov-action@v7 - with: - token: ${{ secrets.CODECOV_TOKEN }} - uses: actions/upload-artifact@v7 if: failure() with: @@ -237,9 +239,10 @@ jobs: - name: Uninstall Modules run: | make uninstall + # no token needed: public repo uses Codecov's tokenless upload via + # GitHub OIDC, which also works for pull requests coming from forks + # (base repo secrets are never exposed to fork-triggered PR runs) - uses: codecov/codecov-action@v7 - with: - token: ${{ secrets.CODECOV_TOKEN }} - uses: actions/upload-artifact@v7 if: failure() with: @@ -305,9 +308,10 @@ jobs: - name: Uninstall Modules run: | make uninstall + # no token needed: public repo uses Codecov's tokenless upload via + # GitHub OIDC, which also works for pull requests coming from forks + # (base repo secrets are never exposed to fork-triggered PR runs) - uses: codecov/codecov-action@v7 - with: - token: ${{ secrets.CODECOV_TOKEN }} - uses: actions/upload-artifact@v7 if: failure() with: @@ -375,9 +379,10 @@ jobs: - name: Uninstall Modules run: | make uninstall + # no token needed: public repo uses Codecov's tokenless upload via + # GitHub OIDC, which also works for pull requests coming from forks + # (base repo secrets are never exposed to fork-triggered PR runs) - uses: codecov/codecov-action@v7 - with: - token: ${{ secrets.CODECOV_TOKEN }} - uses: actions/upload-artifact@v7 if: failure() with: @@ -520,9 +525,10 @@ jobs: - name: Uninstall Modules run: | make uninstall + # no token needed: public repo uses Codecov's tokenless upload via + # GitHub OIDC, which also works for pull requests coming from forks + # (base repo secrets are never exposed to fork-triggered PR runs) - uses: codecov/codecov-action@v7 - with: - token: ${{ secrets.CODECOV_TOKEN }} - uses: actions/upload-artifact@v7 if: failure() with: diff --git a/.github/workflows/macos_tests.yaml b/.github/workflows/macos_tests.yaml index a98615f57..02660c30c 100644 --- a/.github/workflows/macos_tests.yaml +++ b/.github/workflows/macos_tests.yaml @@ -62,9 +62,10 @@ jobs: - name: Uninstall Modules run: | make uninstall + # no token needed: public repo uses Codecov's tokenless upload via + # GitHub OIDC, which also works for pull requests coming from forks + # (base repo secrets are never exposed to fork-triggered PR runs) - uses: codecov/codecov-action@v7 - with: - token: ${{ secrets.CODECOV_TOKEN }} - uses: actions/upload-artifact@v7 if: failure() with: