Tk Source Code

This ticket is meant to support the development of subject A.1 of the proposal
that was sent to the TCLCORE mailing list on 25 Nov 2024 09:53:45. See below for
an excerpt.

The primary goals of the project in this ticket are de-duplication and
collecting of code. Improving overview and intelligibility of the code involved
in this collecting effort are done along the way.

This ticket has assumed the nature of a course log so that people can keep an
overview of the progress.

Development takes place in branch "tk_collect_test_utils".
Timeline: [https://core.tcl-lang.org/tk/timeline?r=tk_collect_test_utils]

The original text from the proposal for these subjects is:

┃ A.1 Collect utility procs
┃ -------------------------
┃ Utility procs are helper procs used by tests. I'm distinguishing global
┃ utility procs from local utility procs. Global utility procs are expected to
┃ be used by multiple test files. Local utility procs are expected to be used
┃ only in the test file that holds the proc definition.
┃
┃ A.1.1 Collect global utility procs
┃ Currently, global utility procs exist in various test files and in the file
┃ constraints.tcl. I propose to move global utility procs and collect them in
┃ a new, separate file "utils.tcl".
┃
┃ A.1.2 Collect test-file-local utility procs
┃ Collect them in a dedicated section somewhere at the top of the test file.
┃
┃
┃ A.2 Collect (more) test constraint definitions in constraints.tcl
┃ -----------------------------------------------------------------
┃ For example: pressbutton, failsOnXQuarz, failsOnUbuntu (the latter two
┃ for the time that they continue to exist ...)
┃
┃
┃ A.3 Make internal structure of test files uniform
┃ -------------------------------------------------


Along the way:
- it appeared to be convenient to include subject A.2 of the proposal into the
  present ticket, which was done/completed.
- it appeared to be better not to carry out subject A.1.2 in the present ticket,
  but to do that together with subject A.3 (in a separate ticket, yet to come).

ADDENDA
=======

After the merge into trunk, several places in the Tk code were discoverd that
could be simplified by collecting or using already collected procs.

These are visible in the Tk repository as branches continued off the original
development branch with names that start with "addendum".

PROJECT COMPLETED
The development branch has been merged into trunk (commit [0723cd322f]).
Thanks for your guidance François.

Closing this ticket.

Yes Erik, you have my approval.

If you are uncomfortable in the technical details, just tell me and I will do the merge. But currently there is no conflict so it's totally straightforward. Don't forget to remove the changes in .github/workflows/*.* before committing though, to close the tk_collect_test_utils branch and to report completion in the present ticket (which should then be closed as well) afterwards.

Very good job Erik, thanks!

PROJECT COMPLETION AND MERGE INTO TRUNK

The few issues that surfaced during the final review have been resolved. As far as
I'm concerned, the project is now complete and ready for merging into trunk.

@François: do you approve of a merge of branch tk_collect_test_utils into trunk?

Yes, that's right Francois. And I saw the post by Nicolas only after I had committed
a fix for that same problem, which I saw earlier on Github CI.

This morning, Github confirmed the correctness of my fix, also on other platforms
than macOS. (Github reports one failure on macOS, but that's the usual test pack-18.2,
which fails intermittently for other reasons than project tk_collect_test_utils,
also on trunk.)

Oh I see, Erik make changes yesterday and both Nicolas and the CI saw the problem.

@Erik: I'm surprised and unsure about why the problem Nicolas saw with spinbox-19.2[01] on his setup got undetected by the CI, do you know why?

@Nicolas: the tests are not executed twice, at least not twice identically. There is one run for the so-called "Deployment" (aka release) version and one run for the "Development" (aka debug) version.

Hi Nicolas,

I don't know why the tests are executed twice in your situation.
Regardless, it's very valuable for this project to see that the Tk test suite now runs without failures.

Thanks.

/Applications/Xcode.app/Contents/Developer/usr/bin/make -C "/Users/nico/daylight/lib/build/tk/macosx/../../build/tk/Development" test TCL_EXE='/usr/local/bin/tclsh9.1' INSTALL_ROOT='' INSTALL_TARGETS='install-binaries install-libraries install-headers install-private-headers install-demos' VERSION='9.1'  
DYLD_FRAMEWORK_PATH="`pwd`:/Library/Frameworks/Tcl.framework:${DYLD_FRAMEWORK_PATH}"; export DYLD_FRAMEWORK_PATH; TCL_LIBRARY=/Users/nico/daylight/lib/build/tcl/library; export TCL_LIBRARY; TK_LIBRARY=/Users/nico/daylight/lib/build/tk/library; export TK_LIBRARY; ./tktest /Users/nico/daylight/lib/build/tk/unix/../tests/all.tcl -geometry +0+0 
Tests running in interp:  /Users/nico/daylight/lib/build/build/tk/Development/tktest
Tests located in:  /Users/nico/daylight/lib/build/tk/tests
Tests running in:  /Users/nico/daylight/lib/build/build/tk/Development
Temporary files stored in /Users/nico/daylight/lib/build/build/tk/Development
Test files sourced into current interpreter
Running tests that match:  *
Skipping test files that match:  l.*.test
Only running test files that match:  *.test
Tests began at Mon Apr 21 13:54:37 CEST 2025
bell.test
Bell should ring now ...
bgerror.test
bind.test
bitmap.test
border.test
busy.test
button.test
canvImg.test
canvMoveto.test
canvPs.test
2025-04-21 13:54:41.461 tktest[70519:3796130] tkMacOSXImage.c:917: XGetImage(): XGetImage does not handle XYPixmaps at the moment.
2025-04-21 13:54:41.473 tktest[70519:3796130] tkMacOSXImage.c:917: XGetImage(): XGetImage does not handle XYPixmaps at the moment.
canvRect.test
canvText.test
canvWind.test
canvas.test
choosedir.test
clipboard.test
clrpick.test
cluster.test
cmds.test
color.test
config.test
cursor.test
dialog.test
embed.test
entry.test
event.test
filebox.test
focus.test
focusTcl.test
font.test
fontchooser.test
frame.test
geometry.test
get.test
grab.test
grid.test
image.test
imgBmap.test
imgListFormat.test
imgPNG.test
imgPPM.test
imgPhoto.test
imgSVGnano.test
listbox.test
main.test
menu.test
menuDraw.test
menubut.test
message.test
msgbox.test
obj.test
option.test
pack.test
packgrid.test
panedwindow.test
pkgconfig.test
place.test
raise.test
safe.test
safePrimarySelection.test
scale.test
scrollbar.test
select.test
send.test
spinbox.test
systray.test
2025-04-21 13:55:37.878 tktest[70519:3796130] A call to OSACopyScriptingDefinition() by NSScriptSuiteRegistry returned an error. Is this application's OSAScriptingDefinition Info.plist entry valid? If its value is 'dynamic', is there an 'ascr'/'gsdf' Apple event handler registered?
testutils.test
text.test
textBTree.test
textDisp.test
textImage.test
textIndex.test
textMark.test
textTag.test
textWind.test
tk.test
this wish doesn't have XIM (X Input Methods) support
unixButton.test
unixEmbed.test
unixFont.test
unixMenu.test
unixSelect.test
unixWm.test
2025-04-21 13:55:52.642 tktest[70519:3796130] The type attribute is ignored on macOS.
2025-04-21 13:55:52.644 tktest[70519:3796130] The type attribute is ignored on macOS.
2025-04-21 13:55:52.654 tktest[70519:3796130] The type attribute is ignored on macOS.
2025-04-21 13:55:52.656 tktest[70519:3796130] The type attribute is ignored on macOS.
2025-04-21 13:55:52.664 tktest[70519:3796130] The type attribute is ignored on macOS.
util.test
visual.test
visual_bb.test
winButton.test
winClipboard.test
winDialog.test
winFont.test
winMenu.test
winMsgbox.test
winSend.test
winWm.test
window.test
winfo.test
wm.test
xmfbox.test

Tests ended at Mon Apr 21 13:56:06 CEST 2025
all.tcl:	Total	10033	Passed	8812	Skipped	1221	Failed	0
Sourced 97 Test Files.
Number of tests skipped for each constraint:
	11	altDisplay
	10	aquaKnownBug
	1	cliboardManagerPresent
	16	colorsFree
	21	defaultPseudocolor8
	10	emptyTest
	126	fonts
	2	haveDISPLAY
	5	haveOtherVisual
	1	havePseudocolorVisual
	8	knownBug
	1	memory
	95	nonPortable
	107	nonUnixUserInteraction
	58	notAqua
	62	nt
	5	pseudocolor8
	10	scriptImpl
	21	testmenubar
	11	testmetrics
	54	testsend
	15	testutils
	7	testwinevent
	54	testwrapper
	1	userInteraction
	1	utfcompat
	342	win
	51	winSend
	115	x11
DYLD_FRAMEWORK_PATH="`pwd`:/Library/Frameworks/Tcl.framework:${DYLD_FRAMEWORK_PATH}"; export DYLD_FRAMEWORK_PATH; TCL_LIBRARY=/Users/nico/daylight/lib/build/tcl/library; export TCL_LIBRARY; TK_LIBRARY=/Users/nico/daylight/lib/build/tk/library; export TK_LIBRARY; ./tktest /Users/nico/daylight/lib/build/tk/unix/../tests/ttk/all.tcl -geometry +0+0 \
	
Tests running in interp:  /Users/nico/daylight/lib/build/build/tk/Development/tktest
Tests located in:  /Users/nico/daylight/lib/build/tk/tests/ttk
Tests running in:  /Users/nico/daylight/lib/build/build/tk/Development
Temporary files stored in /Users/nico/daylight/lib/build/build/tk/Development
Test files sourced into current interpreter
Running tests that match:  *
Skipping test files that match:  l.*.test
Only running test files that match:  *.test
Tests began at Mon Apr 21 13:56:06 CEST 2025
checkbutton.test
combobox.test
entry.test
image.test
labelframe.test
layout.test
notebook.test
panedwindow.test
progressbar.test
radiobutton.test
scale.test
scrollbar.test
spinbox.test
treetags.test
treeview.test
ttk.test
validate.test
vsapi.test

Tests ended at Mon Apr 21 13:56:09 CEST 2025
all.tcl:	Total	591	Passed	576	Skipped	15	Failed	0
Sourced 18 Test Files.
Number of tests skipped for each constraint:
	2	NA
	4	coreEntry
	1	notAqua
	5	nyi
	3	xpnative
/Applications/Xcode.app/Contents/Developer/usr/bin/make test-tk \
	 BUILD_STYLE=Deployment INSTALL_TARGET=install-strip  
/Applications/Xcode.app/Contents/Developer/usr/bin/make -C "/Users/nico/daylight/lib/build/tk/macosx/../../build/tk/Deployment" binaries libraries tktest INSTALL_ROOT='' INSTALL_TARGETS='install-binaries install-libraries install-headers install-private-headers install-demos' VERSION='9.1'  
make[2]: Nothing to be done for `binaries'.
make[2]: Nothing to be done for `libraries'.
make[2]: `tktest' is up to date.
/Applications/Xcode.app/Contents/Developer/usr/bin/make -C "/Users/nico/daylight/lib/build/tk/macosx/../../build/tk/Deployment" test INSTALL_ROOT='' INSTALL_TARGETS='install-binaries install-libraries install-headers install-private-headers install-demos' VERSION='9.1'  
DYLD_FRAMEWORK_PATH="`pwd`:/Users/nico/daylight/lib/build/build/tcl/Deployment:${DYLD_FRAMEWORK_PATH}"; export DYLD_FRAMEWORK_PATH; TCL_LIBRARY=/Users/nico/daylight/lib/build/tcl/library; export TCL_LIBRARY; TK_LIBRARY=/Users/nico/daylight/lib/build/tk/library; export TK_LIBRARY; ./tktest /Users/nico/daylight/lib/build/tk/unix/../tests/all.tcl -geometry +0+0 
Tests running in interp:  /Users/nico/daylight/lib/build/build/tk/Deployment/tktest
Tests located in:  /Users/nico/daylight/lib/build/tk/tests
Tests running in:  /Users/nico/daylight/lib/build/build/tk/Deployment
Temporary files stored in /Users/nico/daylight/lib/build/build/tk/Deployment
Test files sourced into current interpreter
Running tests that match:  *
Skipping test files that match:  l.*.test
Only running test files that match:  *.test
Tests began at Mon Apr 21 13:56:10 CEST 2025
bell.test
Bell should ring now ...
bgerror.test
bind.test
bitmap.test
border.test
busy.test
button.test
canvImg.test
canvMoveto.test
canvPs.test
canvRect.test
canvText.test
canvWind.test
canvas.test
choosedir.test
clipboard.test
clrpick.test
cluster.test
cmds.test
color.test
config.test
cursor.test
dialog.test
embed.test
entry.test
event.test
filebox.test
focus.test
focusTcl.test
font.test
fontchooser.test
frame.test
geometry.test
get.test
grab.test
grid.test
image.test
imgBmap.test
imgListFormat.test
imgPNG.test
imgPPM.test
imgPhoto.test
imgSVGnano.test
listbox.test
main.test
menu.test
menuDraw.test
menubut.test
message.test
msgbox.test
obj.test
option.test
pack.test
packgrid.test
panedwindow.test
pkgconfig.test
place.test
raise.test
safe.test
safePrimarySelection.test
scale.test
scrollbar.test
select.test
send.test
spinbox.test
systray.test
2025-04-21 13:57:10.921 tktest[70570:3798317] A call to OSACopyScriptingDefinition() by NSScriptSuiteRegistry returned an error. Is this application's OSAScriptingDefinition Info.plist entry valid? If its value is 'dynamic', is there an 'ascr'/'gsdf' Apple event handler registered?
testutils.test
text.test
textBTree.test
textDisp.test
textImage.test
textIndex.test
textMark.test
textTag.test
textWind.test
tk.test
this wish doesn't have XIM (X Input Methods) support
unixButton.test
unixEmbed.test
unixFont.test
unixMenu.test
unixSelect.test
unixWm.test
util.test
visual.test
visual_bb.test
winButton.test
winClipboard.test
winDialog.test
winFont.test
winMenu.test
winMsgbox.test
winSend.test
winWm.test
window.test
winfo.test
wm.test
xmfbox.test

Tests ended at Mon Apr 21 13:57:39 CEST 2025
all.tcl:	Total	10033	Passed	8812	Skipped	1221	Failed	0
Sourced 97 Test Files.
Number of tests skipped for each constraint:
	11	altDisplay
	10	aquaKnownBug
	1	cliboardManagerPresent
	16	colorsFree
	21	defaultPseudocolor8
	10	emptyTest
	126	fonts
	2	haveDISPLAY
	5	haveOtherVisual
	1	havePseudocolorVisual
	8	knownBug
	1	memory
	95	nonPortable
	107	nonUnixUserInteraction
	58	notAqua
	62	nt
	5	pseudocolor8
	10	scriptImpl
	21	testmenubar
	11	testmetrics
	54	testsend
	15	testutils
	7	testwinevent
	54	testwrapper
	1	userInteraction
	1	utfcompat
	342	win
	51	winSend
	115	x11
DYLD_FRAMEWORK_PATH="`pwd`:/Users/nico/daylight/lib/build/build/tcl/Deployment:${DYLD_FRAMEWORK_PATH}"; export DYLD_FRAMEWORK_PATH; TCL_LIBRARY=/Users/nico/daylight/lib/build/tcl/library; export TCL_LIBRARY; TK_LIBRARY=/Users/nico/daylight/lib/build/tk/library; export TK_LIBRARY; ./tktest /Users/nico/daylight/lib/build/tk/unix/../tests/ttk/all.tcl -geometry +0+0 \
	
Tests running in interp:  /Users/nico/daylight/lib/build/build/tk/Deployment/tktest
Tests located in:  /Users/nico/daylight/lib/build/tk/tests/ttk
Tests running in:  /Users/nico/daylight/lib/build/build/tk/Deployment
Temporary files stored in /Users/nico/daylight/lib/build/build/tk/Deployment
Test files sourced into current interpreter
Running tests that match:  *
Skipping test files that match:  l.*.test
Only running test files that match:  *.test
Tests began at Mon Apr 21 13:57:39 CEST 2025
checkbutton.test
combobox.test
entry.test
image.test
labelframe.test
layout.test
notebook.test
panedwindow.test
progressbar.test
radiobutton.test
scale.test
scrollbar.test
spinbox.test
treetags.test
treeview.test
ttk.test
validate.test
vsapi.test

Tests ended at Mon Apr 21 13:57:42 CEST 2025
all.tcl:	Total	591	Passed	576	Skipped	15	Failed	0
Sourced 18 Test Files.
Number of tests skipped for each constraint:
	2	NA
	4	coreEntry
	1	notAqua
	5	nyi
	3	xpnative
nico@Mac macosx % 


I don't know if I'm doing it right....
I build Tk as usual which install Tk framework in /Library/Frameworks. Then I cd macosx, then make test.
at this point Tk is rebuilt and as you can see tests are executed twice

++

Hi Nicolas,

Thanks for testing. 

Yes, this was an oversight. I corrected it this morning with commits [767576bafc] and [57a86249f0]. Would you be willing to run the test suite again?

Erik.

Hi Erik,
I did run make test with your branch on macOS 15.4.1 and here's the results:

/Applications/Xcode.app/Contents/Developer/usr/bin/make -C "/Users/nico/daylight/lib/build/tk/macosx/../../build/tk/Deployment" test INSTALL_ROOT='' INSTALL_TARGETS='install-binaries install-libraries install-headers install-private-headers install-demos' VERSION='9.1'  
DYLD_FRAMEWORK_PATH="`pwd`:/Users/nico/daylight/lib/build/build/tcl/Deployment:${DYLD_FRAMEWORK_PATH}"; export DYLD_FRAMEWORK_PATH; TCL_LIBRARY=/Users/nico/daylight/lib/build/tcl/library; export TCL_LIBRARY; TK_LIBRARY=/Users/nico/daylight/lib/build/tk/library; export TK_LIBRARY; ./tktest /Users/nico/daylight/lib/build/tk/unix/../tests/all.tcl -geometry +0+0 
Tests running in interp:  /Users/nico/daylight/lib/build/build/tk/Deployment/tktest
Tests located in:  /Users/nico/daylight/lib/build/tk/tests
Tests running in:  /Users/nico/daylight/lib/build/build/tk/Deployment
Temporary files stored in /Users/nico/daylight/lib/build/build/tk/Deployment
Test files sourced into current interpreter
Running tests that match:  *
Skipping test files that match:  l.*.test
Only running test files that match:  *.test
Tests began at Mon Apr 21 10:06:55 CEST 2025
bell.test
Bell should ring now ...
bgerror.test
bind.test
bitmap.test
border.test
busy.test
button.test
canvImg.test
canvMoveto.test
canvPs.test
canvRect.test
canvText.test
canvWind.test
canvas.test
choosedir.test
clipboard.test
clrpick.test
cluster.test
cmds.test
color.test
config.test
cursor.test
dialog.test
embed.test
entry.test
event.test
filebox.test
focus.test
focusTcl.test
font.test
fontchooser.test
frame.test
geometry.test
get.test
grab.test
grid.test
image.test
imgBmap.test
imgListFormat.test
imgPNG.test
imgPPM.test
imgPhoto.test
imgSVGnano.test
listbox.test
main.test
menu.test
menuDraw.test
menubut.test
message.test
msgbox.test
obj.test
option.test
pack.test
packgrid.test
panedwindow.test
pkgconfig.test
place.test
raise.test
safe.test
safePrimarySelection.test
scale.test
scrollbar.test
select.test
send.test
spinbox.test


==== spinbox-19.20 spinbox widget validation FAILED
==== Contents of test case:

    spinbox .e -validate all  -validatecommand $validateCmd1  -invalidcommand bell  -textvariable ::e  -background red -foreground white
    pack .e
    set ::e nextdata                 ;# previous settings
    .e configure -validatecommand $validateCmd2 ;# prev
    .e validate                     ;# previous settings

    .e configure -validate all
    set ::e testdata
    list [.e cget -validate] [.e get] $::e $validationData

---- Result was:
all testdata testdata {.e -1 -1 testdata nextdata {} all forced}
---- Result should have been (exact matching):
all testdata mydata {.e -1 -1 testdata mydata {} all forced}
==== spinbox-19.20 FAILED



==== spinbox-19.21 spinbox widget validation - bug 40e4bf6198 FAILED
==== Contents of test case:

    spinbox .e -validate key  -validatecommand $validateCmd2  -textvariable ::e
    pack .e
    set ::e origdata
    .e insert 0 A
    list [.e cget -validate] [.e get] $::e $validationData

---- Result was:
key Aorigdata Aorigdata {.e 1 0 Aorigdata origdata A key key}
---- Result should have been (exact matching):
none origdata mydata {.e 1 0 Aorigdata origdata A key key}
==== spinbox-19.21 FAILED

systray.test
2025-04-21 10:07:56.822 tktest[65175:3721882] A call to OSACopyScriptingDefinition() by NSScriptSuiteRegistry returned an error. Is this application's OSAScriptingDefinition Info.plist entry valid? If its value is 'dynamic', is there an 'ascr'/'gsdf' Apple event handler registered?
testutils.test
Test file error: assertion failed: ""bar" ni [info vars bar]"
text.test
textBTree.test
textDisp.test
textImage.test
textIndex.test
textMark.test
textTag.test
textWind.test
tk.test
this wish doesn't have XIM (X Input Methods) support
unixButton.test
unixEmbed.test
unixFont.test
unixMenu.test
unixSelect.test
unixWm.test
util.test
visual.test
visual_bb.test
winButton.test
winClipboard.test
winDialog.test
winFont.test
winMenu.test
winMsgbox.test
winSend.test
winWm.test
window.test
winfo.test
wm.test
xmfbox.test

Tests ended at Mon Apr 21 10:08:25 CEST 2025
all.tcl:	Total	10024	Passed	8810	Skipped	1212	Failed	2
Sourced 97 Test Files.
Files with failing tests: spinbox.test
Number of tests skipped for each constraint:
	11	altDisplay
	10	aquaKnownBug
	1	cliboardManagerPresent
	16	colorsFree
	21	defaultPseudocolor8
	10	emptyTest
	126	fonts
	2	haveDISPLAY
	5	haveOtherVisual
	1	havePseudocolorVisual
	8	knownBug
	1	memory
	95	nonPortable
	107	nonUnixUserInteraction
	58	notAqua
	62	nt
	5	pseudocolor8
	10	scriptImpl
	21	testmenubar
	11	testmetrics
	54	testsend
	6	testutils
	7	testwinevent
	54	testwrapper
	1	userInteraction
	1	utfcompat
	342	win
	51	winSend
	115	x11
DYLD_FRAMEWORK_PATH="`pwd`:/Users/nico/daylight/lib/build/build/tcl/Deployment:${DYLD_FRAMEWORK_PATH}"; export DYLD_FRAMEWORK_PATH; TCL_LIBRARY=/Users/nico/daylight/lib/build/tcl/library; export TCL_LIBRARY; TK_LIBRARY=/Users/nico/daylight/lib/build/tk/library; export TK_LIBRARY; ./tktest /Users/nico/daylight/lib/build/tk/unix/../tests/ttk/all.tcl -geometry +0+0 \
	
Tests running in interp:  /Users/nico/daylight/lib/build/build/tk/Deployment/tktest
Tests located in:  /Users/nico/daylight/lib/build/tk/tests/ttk
Tests running in:  /Users/nico/daylight/lib/build/build/tk/Deployment
Temporary files stored in /Users/nico/daylight/lib/build/build/tk/Deployment
Test files sourced into current interpreter
Running tests that match:  *
Skipping test files that match:  l.*.test
Only running test files that match:  *.test
Tests began at Mon Apr 21 10:08:25 CEST 2025
checkbutton.test
combobox.test
entry.test
image.test
labelframe.test
layout.test
notebook.test
panedwindow.test
progressbar.test
radiobutton.test
scale.test
scrollbar.test
spinbox.test
treetags.test
treeview.test
ttk.test
validate.test
vsapi.test

Tests ended at Mon Apr 21 10:08:28 CEST 2025
all.tcl:	Total	591	Passed	576	Skipped	15	Failed	0
Sourced 18 Test Files.
Number of tests skipped for each constraint:
	2	NA
	4	coreEntry
	1	notAqua
	5	nyi
	3	xpnative
nico@Mac macosx % 

I don't know if this helps...
best regards,
nicolas

FINAL REVIEW

This project is in its last stage of a final review. An invitation for comments to
that effect has been posted on the TCLCORE mailing list on Sun, 6 Apr 2025 23:21:24 +0200.

The check-ins since that invitation carry a tag FINAL_REVIEW.

MAUALLY OR AUTOMATICALLY SETTING THE DIALOG TYPE FOR DOMAIN "dialog"

This post regards commits [5c3431e218] through [c888fa3c11], and [9432c90928].
They reflect the struggle I had deciding between:

Case A: previously, this was done by invoking the utility proc "setDialogType"
in the test file, immediately after having imported the domain "dialog".

Case B: commit [5c3431e218], plus the successive commit, intend to let that be
done automatically when importing the domain "dialog". This commit was backed
out because it appeared to lead to importing a namespace proc "isNative" and a
namespace variable "dialogType" that are never used by the test file.

Eventually I decided to reinstate the automatic determination in case B.

Rationale:

Case A. *requires* the test author to invoke a utility proc. I don't like the
obligatory aspect. Requiring a test author to do something specific besides
importing a domain, is at odds with the intention of the utility procs act as
a service. This just doesn't seem right.

The decisive reason to eventually reinstate the implementation of case B. is
that "convenience for the test author" is paramount. (Note that "convenience
for the test author" was also the consideration that determined the choice
between the various approaches for accessing associated namespace variables.
See posts d.d. 2025-02-10 through 2025-02-25 in this ticket.)

Another reason for choosing case B is that the relative "uncleanliness" of
having a proc or a variable imported that is never used, may just be something
that can be accepted. After all:
- it doesn't harm
- not all test files that import a certain domain use all its utility procs and
  variables anyway; so this was already was accepted.

If it were to become really important to be "perfectly clean", then we can refine
the import strategy later.

COLLECTING GLOBAL TEST CONSTRAINTS

Besides utility procs, this project has also collected test constraints that
were used in multiple test files (a.k.a. global test constraints).

Although this task was originally a separate subject A.2 of the original proposal,
it appeared to be convenient to carry out this task in the present ticket.
This task is now done.

TESTS FOR THE TESTUTILS MECHANISM

The test file "testutils.test" exercises the correct functioning of the exporting, importing and cleanup of utility procs through the proc "testutils",
which is the interface for the test author to the testutils mechanism.

UNIFIED DOCUMENTATION IN "testutils.GUIDE"

A new file "testutils.GUIDE" now contains the unified documentation of the entire
testutils mechanism. The document targets test authors, as well as developers
who carry out maintenance (debugging, improvements otherwise).

Many textual passages that littered the code file testutils.tcl have been moved
to the new document, and the code file refers to it.

THE METHOD OF CHOICE FOR ACCESSING NAMESPACE VARIABLES IN UTILITY DOMAINS: APPROACH A

After comparing and evaluating the the various approaches for accessing namespace
variables in utility domains (see "nsvar_access.xlsx"), Francois and I finally
selected approach A (enhanced with the auto-initialization mechanism).

The by far most important consideration for this choice is "user-friendliness
for test authors".

All methods except approach A are more or less unfriendly in this respect. They
either require the test author to use namespace qualifiers when referring to 
namespace variables associated with utility procs (approaches A2, A3, B and C),
or they require the test author to use access procs for namespace variables
associated with utility procs (approach B), which incurs a level of indirection
for the test author.

Additional arguments of a lesser importance were:
- reduced execution efficiency through overhead of access procs (for approach B)
- risk of pollution of the domain-specific namespace that provides the utility
  procs (for approach C)

The final choice for approach A implies that those test files that were implemented
with approach B, need to be converted to use approach A.

that ought be: 

UPDATED SPREADSHEET FILE "nsvar_access.ods" => "nsvar_access.xlsx"

UPDATED SPREADSHEET FILE "nsvar_access.xlsx" => "nsvar_access.xlsx"

LibreOffice reads that file format too. The file nsvar_access.ods has been
removed.

COMPARING APPROACHES FOR ACCESSING ASSOCIATED NAMESPACE VARIABLES

An effort to compare and contrast the various approaches for accessing
namespace variables associated with utility procs for a specific functional
area (domain), resulted in a table that is attached as spreadsheet file
"nsvar_access.ods".

REMEDY APPROACH A FOR ACCESSING ASSOCIATED NAMESPACE VARIABLES

Some utility procs from specific functional areas store state in a namespace
variable that is also accessed from the namespace in which the tests are
executed (the "executing namespace"). Some tests require such variables
to be initialized.

When such variables are imported into the "executing namespace" through
an "upvar" command, and the test file unsets these variables as part of a
cleanup operation, this results in the deletion of the target variable
inside the specific domain namespace. This, in turn, poses a problem for
the next test file, which presumes that the variable is initialized.

The proc "testutils" (See commit [6052e1790b] for the implementation) deals
with this upvar issue as follows:

If a namespace for a specific functional area holds a proc "init", the
"testutils import xxx" will invoke it to carry out the initialization of
such namespace variables and subsequently imports them into the executing
namespace using "upvar" (import with auto-initialization).
Upon test file cleanup "testutils forget xxx" will remove the imported
utility procs with the associated namespace variables, and unset the upvar'ed
variable in both the source and target namespace, including their link. The
link and initialization will be recreated for the next namespace upon
"testutils import yyy".

Test writers that create a new utility procs that use a namespace variable
that is also accessed by a test file, need to add the initialization
statements to the init proc. Just placing them inside the "namespace eval"
scope for the specific domain (outside the init proc) isn't enough because
that foregoes the importing of the namespace variables and their automatic
re-initialization.

Oh, and the real origin of the change in winDialog-5.9 is [8042288c32] (not [75f61e7a35], which is just a merge commit). Note that the if {![vista?]} in [8042288c32] encompasses the entire test. This was changed a month later in [d878a33451].

About test winDialog-5.9: I agree with you, in [75f61e7a35] the "if" clause was intended to disable the test on Windows versions starting at Vista (that is those returning a tcl_platform(osVersion) >= 6.0). The test now always returns a value that is equal to the expected value. One can either:

1. remove the test on the grounds it is useless

or

2. keep it with its (slightly adapted) comment in the hope some day it becomes useful as a template to adapt when the corresponding feature gets implemented

or

3. rework the test so that the correct result is expected (probably the if {![vista?]} part in [75f61e7a35]) and constrain the test with, say, 'knownBug'.

I would probably go for #2 (with proper explanation in comment), or #3 (ditto).

Thanks for your response François,

I will proceed as suggested/advised. But specifically regarding:

4: I didn't know about the disabled "send" implementation. I won't touch the file winSend.test.

5: Commit [75f61e7a35] does provide a clue:

-- diff start --
 test winDialog-5.9 {GetFileName: file types} -constraints {
     nt testwinevent
 } -body {
-#        case FILE_TYPES:
-
+    #        case FILE_TYPES:
+
     start {tk_getSaveFile -filetypes {{"foo files" .foo FOOF}} -title Foo}
-    then {
-        set x [GetText 0x470]
-        Click cancel
+    # XXX - currently disabled for vista style dialogs because the file
+    # types control has no control ID and we don't have a mechanism to
+    # locate it.
+    if {[vista?]} {
+        then {
+            Click cancel
+        }
+        return 1
+    } else {
+        then {
+            set x [GetText 0x470]
+            Click cancel
+        }
+        return [string equal $x {foo files (*.foo)}]
     }
-    return $x
-} -result {foo files (*.foo)}
+} -result 1
 test winDialog-5.10 {GetFileName: file types: MakeFilter() fails} -constraints {
     nt
 } -body {
-- diff end --

The comment and the "if {[vista?]} { ... " clause were added with this same commit.
That makes me understand that the "if" clause was intended to disable the test.
However, doing that by adapting the test body to prevent a test failure is strange.
That should have been done by constraining the test to not run if the condition "[vista?]" is true.

- Does this logic make you look any different to the matter at hand?
- Do you believe it's all right to let the test exist in its current version?
- Should we contact the committer (dgp) for conclusiveness?

Here is my opinion on your 6 questions.

1. Changing:

-  Tcl_SetVar2(tsdPtr->debugInterp, "tk_dialog", NULL, buf, TCL_GLOBAL_ONLY);
+  Tcl_SetVar2(tsdPtr->debugInterp, "::tk::test::dialog::tk_dialog", NULL, buf, NULL);

is totally fine given that this is advertised as being only used by the test suite (and is not part of the documented public interface). This is stated in the comments of the SetTkDialog function. Interestingly, it says that this is enabled when a test program calls TkWinDialogDebug by calling the test command 'tkwinevent debug 1'. But 'tkwinevent debug 1' is not called anywhere in the Tk repository: the correct spelling is 'testwinevent debug 1'.

Note that comments here and there (e.g. in tkWinDialog.c:156 or 649, or ...) should be updated accordingly if the variable name changes. Perhaps certain function names (SetTkDialog?) should be changed accordingly as well.

2. You can (should) delete this comment. The reference to the Windows TEMP directory is here because tcltest::temporaryDirectory probably used to use this directory in the distant past. All this is very outdated. Juste rely on the temporary directory given by the tcltest::temporaryDirectory command (it knows what it is doing).

3. JE must be Joe English. He is the author of Tile (aka Ttk). He has left the building and resigned from the TCT in January 2018. You can remove this comment, no problem.

4. I have no idea whether there are any plans to (re-)implement "send" on Windows in the future. But I'm not seeing any activity on this currently and for a long time. I'd keep the tests they can always be used at some point later if someone gets interested in providing an implementation. (The comments at the top of this file speaks about tkSend.c which I think does not exist, or does no longer exist). Are you sure there is no implementation? There is a tkWinSend.c file that seems to do just that. What is happening is that [info commands send] returns "", effectively setting testConstraint winSend 0 which skips all tests. There is a TK_SEND_ENABLED_ON_WINDOWS that can be defined to experiment (I didn't try). I know that the man page says "Under Windows, send is currently disabled. Most of the functionality is provided by the dde command instead.", but removal of this code and associated tests I think would need a discussion on the tcl_core list and probably a TIP (IMO).

5. winDialog-5.9 became what it is now by commit [75f61e7a35]. This may give a hint, but apart from analyzing the history of winDialog.test and specifically the commits that changed winDialog-5.9 in that file I have no idea unfortunately.

6. Proc "menubarheight": good catch. I'm responsible for having messed this up. In [d5e602691a], in branch less_tests_constraints created off core-8-6-branch, I have renamed proc menubarheight to testmenubarheight to conform to how other such procs are named in the test suite. Later, in [e78b5c12b6] in trunk, I have added a fallback for that proc to be used by platforms other than macOS. And again later in [49fdd4b212] I have merged trunk in less_tests_constraints. This last commit created the inconsistency. Proc menubarheight must be renamed to testmenubarheight, which I just did in trunk. I leave you merge trunk in your branch, or cherrypick my commit [d3948b1add], it's up to you.

Additional question:

6. In file testutils.tcl, I see a fallback procedure "menubarheight" (relocated
   from constraints.tcl), but I don't see it used anywhere in the code base.
   Can this be removed?

REMAINING ISSUES: ADVICE REQUESTED

I'm approaching the completion of what I intended with this ticket.

Apart from a decision about the approach to take for accessing variables in the
namespaces specific for functional area's, and a final complete review before
merging, there remain a few loose ends and uncertainties that I'm seeking
advice about.

First off, please note/recall that I intend to carry out subject A.1.2 in a
separate branch (and another ticket), so that the present collecting effort
can be handled distinctly.

My questions/issues:

1. When relocating the procs afterbody, ApplyFont, start and then into the
   namespace ::tk::test::dialog, I've left the variable ::tk_dialog in the
   global namespace, because it is set by tkWinDialog.c as ::tk_dialog.

   It doesn't seem difficult to change this at the C-side, something like:

-  Tcl_SetVar2(tsdPtr->debugInterp, "tk_dialog", NULL, buf, TCL_GLOBAL_ONLY);
+  Tcl_SetVar2(tsdPtr->debugInterp, "::tk::test::dialog::tk_dialog", NULL, buf, NULL);

   so that tk_dialog becomes a namespace variable too. Would that be OK?

2. In winDialog.test, there is this comment, which I left in place when
   removing the proc "initialdir" and replacing all calls to it in commit
   [8e94fb8fe3]:

	# What directory to use in initialdir tests. Old code used to use
	# c:/. However, on Vista/later that is a protected directory if you
	# are not running privileged. Moreover, not everyone has a drive c:
	# but not having a TEMP would break a lot Windows programs
	set initialDir [tcltest::temporaryDirectory]

   I am unsure about its usefulness and relevance because the present ticket
   targets Tk 9.0+, for which anything from the Windows Vista era and earlier
   is deprecated. Also, I didn't understand the reason for mentioning TEMP in
   this context. Maybe this comment can be removed?

3. In ttk/validate.test around line 193 there is a comment "New tests" by JE
   (I don't know who that is). I don't think it's useful to have this in the
   code; certainly not anymore after having been there since 2006-10-31
   (no offence!). Personally, I would remove the comment. But social sensitivity
   makes me think: maybe I'm treading on a habit that I don't know of, and JE
   will be angered.

4. All tests in winSend.test are skipped because the Tk command "send" doesn't
   exist on MS Windows. Therefore, currently, this entire test file is
   unused/useless.
   Are there any plans to (re-)implement "send" on Windows in the future?
   Can the entire file be removed ?

5. In file winDialog.test, I've left what remains from test winDialog-5.9 after
   having removed the code that executed on "pre-vista" (commit [216cffb43c]).
   However, I have the impression from the return value, that this remainder
   has no value for Tk:
   It's fixed to return the expected result 1, except if "Click" errors out.
   I'm questioning whether the original intention was to test "Click", but
   I don't know for sure. I'd appreciate your insight here.


Thanks in advance for your insight/opinion/advice.

That's as of commit [d0b6052473]

RELOCATION COMPLETED, TIDY UP TOP LEVEL FILES MAIN.TCL, TESTUTILS.TCL AND
CONSTRAINTS.TCL

As of commit [b0dbfebf96], the raw collecting/relocating/combining of utility
procs is completed. See also the updated file "relocation.ods". The test
suite on Github passes on all platforms.

What remains to be done is:

1. Decide how to access variables in the common namespaces ::tk::test::* from
   test files. Currently this ticket is awaiting opinions on the
   alternative approaches A, A2, B and C (see below).

2. Tidy up top-level files main.tcl, testutils.tcl and constraints.tcl
   (including those spots in test files that were affected by the
   relocating), with a focus on improving overview and intelligibility. Most of
   this work concerns the file "testutils.tcl".

   The tidying up is important because the relevant top level files are used by 
   all test files, and need to be clear for people adding tests. Improving on
   overview and intelligibility encompasses: adding categories/sections to 
   files, code simplification, using (more) appropriate names for procs and 
   variables that are exported, collecting procs that belong together in an
   ensemble, textual improvements on comments.

That's a cunning idea (of using upvar and evaluating inside a proc). Let's
call this "approach A2".

Advantages (relative to approach C):
- Upvar'ed variables (and all other variables created by the test file except
  global variables) are automatically removed when going out of scope at the
  end of the test file.
- References to variables outside the new scope still need to bridge the stack
  level of the extra proc by prepending the relevant variable names with a
  single "::". But that's less distracting compared to approach C, where three
  namespace levels need to be bridged by prepending the relevant variable names
  with "::tk::test::xxxx::", where xxxx is the name of the relevant functional
  area.

Regarding the suggested code for evaluating inside a proc:

    proc thetests {
    upvar ...
    test entry-x.y {...}
    ...
    }
    thetests

If we decide to do this, we could consider to use the flexibilty and 
expressiveness of the Tcl language to make a control structure that encapsulates 
the awkwardness in this code:

A. In all.tcl add:

	# evalTestFileInProc --
	#
	#	Evaluates the file that makes the call to this proc inside its
	#	own stack level. This allows variables upvar'ed in the test file
	#	to be cleaned up.
	#
	proc evalTestFileInProc {} {
	    if {[info level] == 1} {
		upvar 1 argv argv
		source [info script]
	    }
	}

B. In each test file, add as the very first command:

	evalTestFileInProc

   which makes test file being evaluated, but only at stack level 1, i.e. 
   inside this proc.

Hmmm, I'm mixed.

For sure approach B, i.e. using getters/setters, in files such as textDisp.test would clutter the (already cluttered) expected results.

In contrast, approach C looks promising but at the cost of calls to things like [namespace current]::x which are rather counter-intuitive and would probably lead to errors when writing new tests. Approach C would rise the access bar to newcomers writing tests I think.

Remains approach A. Indeed the manual says there is no way to unset an upvar variable except by exiting the procedure in which it is defined. Could we then circumvent this by enclosing all tests of a test file inside a proc, something like (with comments exmplaining why there is a proc):

proc thetests {
  upvar ...
  test entry-x.y {...}
  ...
}
thetests

Could Tcl folks drop their thoughts here please, and give advice to Erik? I'd like to get other opinions and ideas. Thanks!

ACCESS COLLECTED NAMESPACE VARIABLES: EVAL TEST FILE IN SPECIFIC NAMESPACE

While considering/reviewing the alternative approaches to accessing
variables in a specfic namesace from within a test file, I'd like a third
method to be considered:

C. Evaluate each test file inside the specific namespace whose utility procs
   and variables it uses

Advantages:
- no need for importing utility procs, test files keep their original
  invocations (utility procs were relocated in the very same namespace)
- no need for access procs

Drawbacks:
Anything that pokes into another namespace (including the global namespace),
but which is accessed relative to the global namespace [*], needs to be made
fully qualified.

    [*] names of wait variables, text variables, traced variables,
        traced procs, ...

I've implemented this approach in an example for entry.test, named
"entry.nseval.test" (attached). If you want to run this test, then
beware to also make this change in testutils.tcl:

--- tests/testutils.tcl
+++ tests/testutils.tcl
@@ -463,11 +463,11 @@


 namespace eval ::tk::test::entry {
     # For trace add variable
     proc override args {
-       global x
+       variable x
        set x 12345
     }

     # Procedures used in widget VALIDATION tests
     proc validateCommand1 {W d i P s S v V} {

With:
        "An obvious advantage is ..."

I meant:

        "An obvious advantage of using upvar is ..."

ACCESS COLLECTED NAMESPACE VARIABLES: USE UPVAR OR AN ACCES PROC ?

Some utility procs access a global variable. After relocation these variables
are turned into a variable in the specific namespace to which the proc is
relocated. Therefore, accessing these variables from within the original
test file needs to be adapted also. There are two approaches to accessing these
relocated variables:
A. use "upvar" to move them into the global namespace in the test files from
   where they originated
B. inside the specific namespace, define a variable-specific access proc, which
   is imported into the global namespace by the originating test file, just as
   the relocated utility procs.

In the beginning of the relocation process I considered both approaches. I
then discarded approach A for the following reason:
Upvar'ed variables cannot be cleaned up (i.e. "unset") in the originating
test file without unsetting the target variable in the specific namespace.
And that would clobber any initialization of those variables inside the
specific namespace.

Therefore, I chose approach B: use access procs.

Now that I'm carrying out the relocation of utility procs from the test files
textDisp.test and textWind.test, I'm confronted with variables that are
referenced many times (some several hundreds) in these test files:

    fixedFont (20 and 8 occurrences respectively)
    fixedWidth (266 and 24 occurrences respectively)
    fixedHeight (322 and 53 occurrences respectively)

Replacing all these occurrences with a call to an access proc, like:

-	set x $fixedHeight      ;# simple variable reference
+	set x [fixedHeight get] ;# a call to the access proc "fixedHeight"

seems to me a too big price to pay w.r.t. execution efficiency, given the
sheer number of procedure calls.

An obvious advantage is that the references to such variables in the original
test files need not be changed, and we keep the original, simpler code which is
somewhat better readable.

Also, there is another small advantage of simply using upvar instead of an
access proc for cases that are not standard. For example:
- the access proc needs another method besides "get" and "set"
- the acces proc accepts more than one value for setting/appending to the
  namespace variable.
These cases need a hand-crafted access proc instead of a standard instance.

All in all, I'm now favouring the use of "upvar" instead of using acces procs.

As an example, I've carried out the relocation of the utility procs bo, xchar,
xw and yline from the test files textDisp.test and textWind.test using this
approach (commit [94795a28]).

At the current stage of relocating, the usage of access procs is as follows
(see also testutils.tcl):

Namespace              Access procs        Test files
-----------------------------------------------------------
::tk::test::dialog     dialogTestFont      choosedir.test, clrpick.test,
                                           filebox.test, msgbox.test
::tk::test::entry      vaildationData      entry.test, spinbox.test,
                                           validate.test
::tk::test::scroll     scrollInfo          entry.test, spinbox.test,
                                           textDisp.test
::tk::test::select     abortCount, pass    select.test, unixSelect.test
                       selInfo, selValue

I don't mind the work of reverting/converting back from approach B to A
for the entire test suite. But before doing so, I'd appreciate explicit
review of this choice, specifically w.r.t. the balance of advantages and
drawbacks of both methods A and B.

UPDATE FILE "relocation.ods"

This file will be updated once in a while when the relocation work makes clear that some previous information in that file is incorrect or if there is new relevant information.

Update is now attached to this ticket.

TRANSFER COLLECTING OF TEST-FILE-LOCAL UTILITY PROCS TO ANOTHER SUBJECT/TICKET

Working on the present ticket/subject made me realize that the actual work of collecting utility procs within a test file is a job that fits better into the following subject of my main proposal:

    A.3.1 Divide test scripts in explicit sections
    ----------------------------------------------

    (See below for the original text for the entire subject A.3)

This is because definitions of utility procs are not wildly scattered throughout a test file. Most of the "collecting work" within a test file consists of just supplying an appropriate section header at the right place in a test file, and possibly changing the order of sections. That's the explicit goal of subject A.3.1.

This means that the present ticket only concerns itself with relocating utility procs from  category B + those from category C that may be combined.

--
Original text for subject A.3 :

A.3 Make internal structure of test files uniform
-------------------------------------------------

A.3.1 Divide test scripts in explicit sections, each marked with a uniform
      section comment like:

      #
      # LOCAL TEST CONSTRAINTS
      #

      DISCUSSION: This idea targets an ideal situation, but I'm unsure whether
      it is feasible to maintain once the sections have been created.
      I'm unsure whether the degree of discipline, that it requires from (all)
      Tk maintainers, to put things where they belong can be "enforced".

      Sections envisaged for now:
      - file header comment containing general description of functionality and
        copyright statements, like the current state.
      - loading and configuration of package tcltest
      - definition of local test constraints
      - definition of local utility procs (see also A.1.2)
      - tests, possibly interspersed with setup and cleanup code for groups
        of related tests such as in canvas.test

A.3.2 Prepend each individual test with a single blank line (two consecutive
      newlines) in all test files. This makes it more easy for the human eye
      to quickly see where a new test starts.

ADDITIONAL ANALYSIS OF UTILITY PROCS CATEGORY C

The spreadsheet file "relocation.ods" has been extended with a worksheet that
holds a manual analysis of utility procs in category C. 

The purpose of this analysis is to identify procs that have a similar
purpose/implementation but different names. These may be combined and
subsequently relocated just as procs from category B.

1. Re: daily automatic build and test:

   Thanks for arranging this for me, and for the pointers to
   information. Will have a look.

2. Re: division of responsibilities:

   Indeed this argument:

> what value could the scroll command prefix have other
> than [list scrollInfo set] since it is used in a
> -x|yscrollcommand ?

   is something that I did not consider. On purpose because I don't
   oversee the matter of useful choices for the relevant test files.
   I'm going give this argument some more thought before deciding.

1. Re: daily automatic build and test:

Added in [370d4d810f]. It's actually not really a daily automatic build/test. To be more precise this will trigger once a day (at the time the Github repository is sync'ed with our main fossil repo, that is ~8 am UTC) if and only if the branch has new commits (see on: push: branch: ... in the .yml files) since last time it ran. There are other possible schemes, but I think this one fits well the need. Documentation of so-called "Github Actions" is here.

Note that when your branch will be merged back into trunk (when your work is finished), one should remember to revert commit [370d4d810f] that made changes in the three .yml files (keeping these changes would not harm, but they are not needed in trunk and would clutter the files unnecessarily).

The CI execution results are here, where you probably want to filter according to the branch you're interested in, i.e. results for branch tk_collect_test_utils. If you connect to that page at the time the Github repo syncs with the fossil repo you can even see the build and tests happening live.

2. Re: division of responsibilities:

I see your point I think. The test file certainly can remain responsible for defining the scroll command prefix that it wants to use, however what value could the scroll command prefix have other than [list scrollInfo set] since it is used in a -x|yscrollcommand ? I believe this assignment pertains to file testutils.tcl but your mileage may vary on this and at the end of the day I have no problem with you deciding.

Typo: "test_utils.tcl" ought be "testutils.tcl"

Thanks for your review François. Here is my preliminary response:

            Regarding:
            I would strongly advocate we add daily automatic build and test of the
            [https://core.tcl-lang.org/tk/timeline?r=tk_collect_test_utils|tk_coll
            ect_test_utils] branch at CI, so that possible regressions on any
            platform can be timely detected. I can arrange that for you, or you
            can do it (see an example I did today for another branch
            [b2ae7b8557|here].

Seems surely a good thing to do. Could you please arrange this for me?

Normally, I would try to carry this out myself, but I have no understanding
of the workings of the CI system. Especially the "daily automatic" aspect
(as opposed to scheduling a single, manual run, or an automatic one with another
frequency). I’ve looked at your example, and of course I can imitate what
you did. But then I still wouldn’t understand how often, or under which conditions a
new test would run, and I'd absolutely hate to make a mistake that brings about
unduly costs (money or computing resources).

Also, I don't know to inspect the results of CI tests. What I've seen thus far from
the web interface for Tk was not intelligible for me. Could you please point
me to documentation where the above things are explained?


            Regarding
            [https://core.tcl-lang.org/tk/artifact/c6bf9327?ln=50,55|this]: the
            implementation of tcltest::loadTestedCommands is in tcltests.tcl (in
            the Tcl source repository), you can see how it works there and decide
            whether the comments in main.tcl need to be changed or not.

Right. I will sort this out.


            Regarding
            [https://core.tcl-lang.org/tk/artifact/fb2e3ad0?ln=277,294|proc
            scrollInfo] (the "intricate case" you named): I would suggest a
            default switch clause to be added (triggering an error), in order to
            catch possible typos and other mistakes in the calls to it. Up to you,
            you decide.

I agree, and will add that.


            Regarding [ef535b72c6]: I understand the purpose, however defining
            <code>set scrollCmdPrefix [list scrollInfo set]</code> in several
            files looks odd since it seems to go against the factorization goal of
            your branch. Could <code>scrollCmdPrefix</code> perhaps be a variable
            from the <code>::tk::test::scroll</code> namespace instead?

Of course there is no technical problem in doing this assignment:

      set ::tk::test::scroll::scrollCmdPrefix [list scrollInfo set]

However, I'm uncomfortable with that from a perspective of division of responsibilities:
- doing this assignment in a test file would suggest that any test file is at liberty to
  make changes to what's been collected (in a namespace) on behalf of multiple (other)
  test files. This is contrary to the intent of test_utils.tcl assuming responsibility/control
  for that.
- doing this assignment in the file test_utils.tcl and thus letting that file define the
  command prefix for test files feels overly “paternalistic”. I'd like the test file
  to remain responsible for defining the scroll command prefix that it wants to use.

I feel that the current situation reflects the right division of responsibilities, but I
don't mind being convinced otherwise. Please let me know if you have more/other arguments
regarding this division of responsibilities.

Regardless, it may be a good idea to be explicit about this division of responsibilities
in an explanatory comment in the file test_utils.tcl.

At commit [ef535b72] (that is tag prototype1), here are my comments.

Overall, all what you did looks very good to me. I have run the test suite on Windows and everything passes (same as in trunk). That said, I would strongly advocate we add daily automatic build and test of the tk_collect_test_utils branch at CI, so that possible regressions on any platform can be timely detected. I can arrange that for you, or you can do it (see an example I did today for another branch [b2ae7b8557|here].

Regarding this: the implementation of tcltest::loadTestedCommands is in tcltests.tcl (in the Tcl source repository), you can see how it works there and decide whether the comments in main.tcl need to be changed or not.

Regarding proc scrollInfo (the "intricate case" you named): I would suggest a default switch clause to be added (triggering an error), in order to catch possible typos and other mistakes in the calls to it. Up to you, you decide.

Regarding [ef535b72c6]: I understand the purpose, however defining set scrollCmdPrefix [list scrollInfo set] in several files looks odd since it seems to go against the factorization goal of your branch. Could scrollCmdPrefix perhaps be a variable from the ::tk::test::scroll namespace instead?

All in all, everything looks nice and well to me, congratulations and keep up the good work!

TAG FOR PROTOTYPE

After having indicated the checkin for the prototype in a previous post, I committed a few improvements. To avoid confusion about which checkin is meant to be exercised as a prototype, I applied the tag "prototype1" to the pertaining checkin.

Uhmmm:

    "the infrastructure for relocated of utility procs"

That should be:

    "the infrastructure for relocated utility procs"

REQUEST FOR REVIEW

This project has now come to a point where review matters.

Current state of the project
----------------------------
- the plan for relocation of utility procs has been detailed and explained in
  several posts in this ticket.
- the development branch contains:
  - the infrastructure for relocated of utility procs
  - all relocations of utility procs from "constraints.tcl" (category A )
  - two selected relocations from category B.
  The current tip [d869327e] serves as a prototype.

I'm asking for a review of the relocation plan. The prototype implementation
is there to be exercised/inspected.

I welcome wishes, advice, preferences, recommendations, etc.
And I'll gladly answer any questions in case something wasn't clear.

But if there is anything about this project that isn't worth the effort, or
that would stand in the way of a final merge, I'd very much like to know that
now rather than later.

I'd be grateful for your thoughts,
Erik
--

PROTOTYPE

A prototype implementation was created as a proof-of-concept for this project.

Besides the relocations from "constraints.tcl" (which were already completed),
the prototype holds two additional relocations of utility procs from category B
(cf "relocation.ods"). These relocations exercise the use of namespaces for
relocated utility procs, as explained in the post from 2024-12-19 08:59:31.

The two example relocations are a straightforward one and a more intricate one:

- proc errHandler (select.test and unixSelect.test), straightforward relocation

- proc scroll (entry.test, scrollbar.test, spinbox.test, textDisp.test), a
  somewhat intricate case:

    This proc accesses a global variable that is also accessed outside proc
    scroll at various places in the test file. The intricacy involved a bit of
    rewrite of the original proc to handle both reading and writing of the
    global variable (=> namespace variable after relocation).

The prototype is ready for inspection/exercising at checkin [d869327e]

FURTHER ANALYSIS FOR CATEGORY B UTILITY PROCS

An additional manual inspection was carried out for utility procs in category B.
to determine:
- whether the multiple occurrences of a proc definition have similar
  purposes/arguments/implementations so that they may be combined into a
  single proc that covers all use cases when moving the definition to
  "testutils.tcl".
- the namespace in which the relocated proc shall reside.
- any intricacies caused by access of global variables by the utility proc, as
  well as outside the proc at various locations in the test script.

The result of the manual inspection is placed in the spreadsheet file
"relocation.ods" (attached).

ADDITIONAL STRUCTURE FOR RELOCATED PROCS: NAMESPACES PER FUNCTIONAL AREA

Besides relocating these procs to the file "testutils.tcl", I intend to put
them in namespaces:

- the namespaces names will correspond to the functional areas that are
  distinguished similarly to what the names of test files do. Relocated
  utility procs that are specific for a functional area will then be imported on
  demand by test files, and cleaned up after usage.
- the namespaces for specific functional areas are children of the ::tk::test
  namespace. Generic utility procs will go into ::tk::test.

This usage of namespaces is done to facilitate overview, to prevent name
collisions and to be able to perform cleanup of all imported utility procs for
a specific functional area with a single command. Note that name collisions
already exist in the current situation, for example in the proc ::getsize,
defined in "geometry.test" and "listbox.test" on the one hand, and in
"unixFont.test" and "winFont.test" on the other hand.

MODIFIED INFRASTRUCTURE FOR LOADING TOP-LEVEL SCRIPTS

Each test file loads by default some top-level code in a file defined by the
test option -loadfile, before starting its tests. Currently, this is the file
"constraints.tcl".

Looking at the contents of this top-level script, it carries out four different
tasks:
a. setup for the root window and Tk application
b. loading and confuring of the tcltest test harness
c. defining global utility procs
d. defining test constraints

So, "constraints.tcl" is overloaded with much more functionality than what the
file name suggests. Considering these functionalities, file names, and the fact
that the option -loadfile accepts just a single file, I propose to reorganize
the loading of top-level scripts as follows:

Rename the top-level file to "main.tcl" and let it carry out:
1. the tasks a. and b. directly, itself
2. the tasks c. and d. by source'ing a separate file for each task, where these
   files do no more than what their name suggests.

This lead to the following file organization:

main.tcl (a+b) ----
                   |
                    ---- testutils.tcl (c)
                   |
                    ---- constraints.tcl (d)

In the development branch I made the necessary changes for this infrastructure,
and moved the code blocks for the different tasks to their proper destination.
See checkins [a459c7d075] through [8b5ff698f8].

This post presents the results of an analysis of utility procs in the Tk
test suite.

ANALYSIS AND CATEGORIZATION
===========================
The attached text file "procinfo.txt" holds the results of an analysis of
utility procs in the Tk test suite. It lists utility procs in the Tk test suite
and divides them into categories that are to be handled differently when
relocating/collecting procs:

A. Utility procs, collected in "constraints.tcl"
   These utility procs are all defined inside a namespace "::tk::test" or
   "::tk::test::bg" and exported/imported from there. They will be moved to a
   new file "testutils.tcl". The same mechanism as for the file constraints.tcl
   will be used to make the Tk test suite source "testutils.tcl".

B. Utility procs, defined in multiple test files
   These are candidates for being moved to the new file "testutils.tcl" (see
   below for the intention to use namespaces).
   But a more thorough check of each proc is needed first to determine whether
   it's feasible to perform a relocation. Considerations for this check are:
   - do the occurrences of a proc across different test files have the same
     purpose (or at least a similar purpose so that can be combined)?
   - how intricate is the usage of these procs in the test script?
   - ...

C. Utility procs defined in a single test file
   These procs will remain in the test file where they are, but they will be
   collected in a separate, contiguous section at the top of the file (except
   if the proc is being redefined in the test file)

D. Utility procs for the Tk visual test
   These procs have already been collected in files that themselves don't
   contain any tests:

	arc.tcl bevel.tcl butGeom.tcl butGeom2.tcl canvPsArc.tcl \
	canvPsBmap.tcl canvPsGrph.tcl canvPsImg.tcl cmap.tcl

   No further degree of collecting seems useful for this category. For now, I
   have no intention to touch these procs.

The categorization of procs in the file procinfo.txt is the result of an
automated procedure, carried out by the attached script "analyse.tcl".


TOOLS
=====
Attached, you find the tools to generate the categorized list in procinfo.txt:
  - Makefile
  - analyse.tcl

Notes:
- the generation is done on Linux. If you want to carry it out on a
  non-unix platform, you need to adapt this recipe.
- the two files need to be placed in the same (project) directory.
- the  following setting in the Makefile needs to be adjusted to your personal
  situation: TK_TESTS_DIR. It needs to point to the location of the tests
  directory (of a Tk source tree) which is to be used for the inspection.
- the analysis employs the TclDevKit program tclchecker to generate a cross
  reference from the files in the tests directory. You need to download/install
  this program yourself[*] so that it launches when issuing "tclchecker".

To generate/reproduce the results in file procinfo.txt, issue in the project
directory:

    "make procinfo"

Or, if you lack make, issue:

    "tclsh analyse.tcl xref; tclsh analyse.tcl procinfo"

--
[*] Links for the tclchecker program:
    - https://sourceforge.net/projects/tclpro/
    - https://www.activestate.com/platform/supported-languages/tcl/