Menu
▾
▴
#628 Multiple test failures running tcltest outside build tree
* May be partially related to #3545367 'DDE test failures' *
If I build and install tclsh85, then copy tcltest to the install's bin folder, and run 'tcltest path/to/tcl/tests/all.tcl' from there, then the following tests fail (that do not fail if run via 'nmake test'). The issues are most likely related to the difference between build-tree and install-tree paths.
Build is a 32-bit tclsh from tag 'core-8-5-12' (checkout [1cc0f68d]), build using MSVC2010, running on Windows 7 64-bit.
==== filesystem-7.1 load from vfs FAILED
==== Contents of test case:
# This may cause a crash on exit
set dir [pwd]
cd [file dirname [info nameof]]
set dde [lindex [glob *dde*[info sharedlib]] 0]
testsimplefilesystem 1
# This loads dde via a complex copy-to-temp operation
load simplefs:/$dde dde
testsimplefilesystem 0
cd $dir
set res "ok"
# The real result of this test is what happens when Tcl exits.
---- Test generated error; Return code was: 1
---- Return code should have been one of: 0 2
---- errorInfo: no files matched glob pattern "*dde*.dll"
while executing
"glob *dde*[info sharedlib]"
("uplevel" body line 5)
invoked from within
"uplevel 1 $script"
---- errorCode: NONE
==== filesystem-7.1 FAILED
Note that this issue is DDE-related, and the DDE tests that fail in bug 3545367 pass when run outside the build tree.
==== msgcat-0.7 locale initialization from environment variables {} FAILED
==== Contents of test case:
i eval msgcat::mclocale
---- Result was:
en_za
---- Result should have been (exact matching):
c
==== msgcat-0.7 FAILED
==== registry-6.21 GetValue: very long value names and values FAILED
==== Contents of test case:
registry set HKEY_CURRENT_USER\\TclFoobar [string repeat k 199] [string repeat x 199] multi_sz
set result [registry get HKEY_CURRENT_USER\\TclFoobar [string repeat k 199]]
registry delete HKEY_CURRENT_USER\\TclFoobar
set result
---- Test generated error; Return code was: 1
---- Return code should have been one of: 0 2
---- errorInfo: invalid command name "registry"
while executing
"registry set HKEY_CURRENT_USER\\TclFoobar [string repeat k 199] [string repeat x 199] multi_sz"
("uplevel" body line 2)
invoked from within
"uplevel 1 $script"
---- errorCode: NONE
==== registry-6.21 FAILED
==== safe-14.1 Check that module path is the same as in the master interpreter [Bug 2964715] FAILED
==== Contents of test case:
set tm {}
foreach token [$i eval ::tcl::tm::path list] {
lappend tm [dict get [set ::safe::S${i}(access_path,map)] $token]
}
return $tm
---- Result was:
C:/User/Tcl_BUILD/tcl85/Release_8-5-12_VC10/lib/tcl8/site-tcl C:/User/Tcl_BUILD/tcl85/Release_8-5-12_VC10/lib/tcl8/8.0 C:/User/Tcl_B
UILD/tcl85/Release_8-5-12_VC10/lib/tcl8/8.1 C:/User/Tcl_BUILD/tcl85/Release_8-5-12_VC10/lib/tcl8/8.2 C:/User/Tcl_BUILD/tcl85/Release
_8-5-12_VC10/lib/tcl8/8.3 C:/User/Tcl_BUILD/tcl85/Release_8-5-12_VC10/lib/tcl8/8.4 C:/User/Tcl_BUILD/tcl85/Release_8-5-12_VC10/lib/t
cl8/8.5 C:/User/Tcl_BUILD/tcl85/Release_8-5-12_VC10/lib/tcl8/8.4/platform
---- Result should have been (exact matching):
C:/User/Tcl_BUILD/tcl85/Release_8-5-12_VC10/lib/tcl8/site-tcl C:/User/Tcl_BUILD/tcl85/Release_8-5-12_VC10/lib/tcl8/8.0 C:/User/Tcl_B
UILD/tcl85/Release_8-5-12_VC10/lib/tcl8/8.1 C:/User/Tcl_BUILD/tcl85/Release_8-5-12_VC10/lib/tcl8/8.2 C:/User/Tcl_BUILD/tcl85/Release
_8-5-12_VC10/lib/tcl8/8.3 C:/User/Tcl_BUILD/tcl85/Release_8-5-12_VC10/lib/tcl8/8.4 C:/User/Tcl_BUILD/tcl85/Release_8-5-12_VC10/lib/t
cl8/8.5
==== safe-14.1 FAILED
On tcl 8.6 (trunk build, MSVC10, Windows 7) the filesystem error is present but slightly different:
(1) Running tcltest.exe outside the build tree sees filesystem-7.1.1 fail with 'no files matched glob pattern "*dde*.dll"' and filesystem-7.1.2 fail (pattern "tclreg*.dll").
(2) Running tclsh86t.exe outside the build tree against filesystem.test does not produce these errors.
filesystem-7.1 (7.1.1 and 7.1.2 on tcl 8.6) fail due to an invalid assumption about the location of the dde and register dlls. In the build/test tree these are adjacent to the tcltest.exe; in the install tree they are in the package directory. No suggested fix yet.
registry-6.21 fails because of a missing 'reg' constraint. registry and dde tests are being skipped when running outside the build environment for other reasons - I'm working on a resolution for that.
Under Tcl 8.6 (and probably 8.5) there are additional differences relating to skipped tests: all dde and registry tests are being skipped when run from the install path as they rely on ::ddelib and ::reglib variables being present and correctly defined.
The follow patch fixes this problem, as well as the related registry-6.21 problem:
--- library\dde\pkgIndex.tcl
+++ library\dde\pkgIndex.tcl
@@ -1,7 +1,9 @@
if {([info commands ::tcl::pkgconfig] eq "")
|| ([info sharedlibextension] ne ".dll")} return
if {[::tcl::pkgconfig get debug]} {
- package ifneeded dde 1.4.0b1 [list load [file join $dir tcldde14g.dll] dde]
+ set ::ddelib [file join $dir tcldde14g.dll]
} else {
- package ifneeded dde 1.4.0b1 [list load [file join $dir tcldde14.dll] dde]
+ set ::ddelib [file join $dir tcldde14.dll]
}
+# ::ddelib is required by windde.test
+package ifneeded dde 1.4.0b1 [list load $::ddelib dde]
--- library\reg\pkgIndex.tcl
+++ library\reg\pkgIndex.tcl
@@ -1,9 +1,9 @@
if {([info commands ::tcl::pkgconfig] eq "")
|| ([info sharedlibextension] ne ".dll")} return
if {[::tcl::pkgconfig get debug]} {
- package ifneeded registry 1.3.0 \ - [list load [file join $dir tclreg13g.dll] registry]
+ set ::reglib [file join $dir tclreg13g.dll]
} else {
- package ifneeded registry 1.3.0 \ - [list load [file join $dir tclreg13.dll] registry]
+ set ::reglib [file join $dir tclreg13.dll]
}
+# ::reglib is required by registry.test
+package ifneeded registry 1.3.0 [list load $::reglib registry]
--- tests\fCmd.test
+++ tests\fCmd.test
@@ -33,7 +33,8 @@
} on error {} {
# try the location given to use on the commandline to tcltest
::tcltest::loadTestedCommands
- load $::reglib Registry
+ catch { load $::reglib Registry }
+ package require registry
}
testConstraint reg 1
}
--- tests\registry.test
+++ tests\registry.test
@@ -22,7 +22,8 @@
if [catch {load {} Registry; set ::reglib {}}] {
# try the location given to use on the commandline to tcltest
::tcltest::loadTestedCommands
- load $::reglib Registry
+ catch { load $::reglib Registry }
+ package require registry
}
testConstraint reg 1
}
@@ -505,7 +506,7 @@
registry delete HKEY_CURRENT_USER\\TclFoobar
set result
} "foo ba r baz"
-test registry-6.21 {GetValue: very long value names and values} {pcOnly} {
+test registry-6.21 {GetValue: very long value names and values} {pcOnly reg} {
registry set HKEY_CURRENT_USER\\TclFoobar [string repeat k 16383] [string repeat x 16383] multi_sz
set result [registry get HKEY_CURRENT_USER\\TclFoobar [string repeat k 16383]]
registry delete HKEY_CURRENT_USER\\TclFoobar
--- tests\winDde.test
+++ tests\winDde.test
@@ -20,7 +20,8 @@
if [catch {load {} Dde; set ::ddelib {}}] {
# try the location given to use on the commandline to tcltest
::tcltest::loadTestedCommands
- load $::ddelib Dde
+ catch { load $::ddelib Dde }
+ package require dde
}
testConstraint dde 1
}] {
Patch for registry-6.21 and skipped dde/reg tests
I don't think it's a good idea to set the global
:ddelib and/or ::::reglib variables from inside
a pkgIndex.tcl file. The following lines in
makefile.vc are supposed to take care of that:
$(TCLTEST) "$(ROOT)/tests/all.tcl" $(TESTFLAGS) -loadfile << > tests.log
set ::ddelib [file normalize $(TCLDDELIB:\=/)]
set ::reglib [file normalize $(TCLREGLIB:\=/)]
<<
Maybe there's something wrong in those lines.
The lines in the Makefile only take care of it in the build environment. There are many excellent reasons to run tests in the install environment. One is to test that packages can actually be located in the install (there have over the years been semi-regular bugs preventing this), another is to be able to provide Win32 binaries for Tcl 8.6b3 that can be used in widespread testing (i.e. running the test suite in multiple production environments) without the need for a build environment.
The tests as currently written expect ::ddelib and ::reglib to be available in order to communicate the location of the DLLs to child interpreters.
There are three approaches to making the tests work in build and install environments:
(1) Set ::ddelib and ::reglib in the install environment. The patch I made is lacking a check -- the vars should not be modified if they already exist. Apart from this the solution works.
(2) Remove the use of ddelib/reglib from the tests, replace with "package require" statements, and have the Makefile provide the appropriate 'package ifneeded' command. This is tricky to get right in the build environment because the 'package require' must happen before ::tcltest::loadTestedCommands to handle static extensions, but will then have set up invalid 'package ifneeded' from the pkgIndex in library/dde.
(3) Modify pkgIndex to use ddelib if present, else to use the default logic ([file join $dir tcldde14.dll]).
Please select an approach (or suggest an alternative), and I'll re-do the patch accordingly.
More on my previous comment:
I can't do (2) without someone looking over the tests and deciding on if it changes the intention of the test.
(3) is actually the extra check required in (1).
So there are only two options: the tests must get ddelib/reglib from somewhere, or must be modified to use 'package require' only.
It would be easy to move ddelib/testlib into a array var in the ::tcl namespace to avoid polluting the global namespace.
And I forgot to add:
(a) In order to work in the install environment the test script must also try 'package require' at some point to force the pkgIndex.tcl to load.
(b) The order of attempts _must_ be: (i) static extension; (ii) ddelib/reglib; (iii) package. This is to prevent accidental loading of a dll from some location other than the one specified by the test script.
(c) clock.test appears to do the right thing (tests pass in build and install environments) by using the following logic:
if {[catch {package require registry 1.1}]
&& [catch {load {} Registry}]
&& [catch {
::tcltest::loadTestedCommands
load $::reglib Registry
}]} {
namespace eval ::tcl::clock {variable NoRegistry {}}
}
This could end up loading the wrong dll though, and which it works for tests involving register it doesn't help with windde.test which uses $ddelib to load directly into slave interps.
(d) Proposed change (in all locations using ddelib and/or reglib):
if { [catch { load {} Register }]
&& [catch { ::tcltest::loadTestedCommands ; load $::reglib Registry }]
&& [catch { pacakge require registry }] } {
# extension absent
}
This will still need the 'package require' to set up a var (like ::ddelib) to indicate the actual DLL.
Sounds good
Updated patch for registry-6.21 and skipped dde/reg tests
Updated td-3549770-3.patch uploaded (apply to trunk, not a delta from the first patch):
- clock.test, fCmd.test, registry.test and windde.test updated to employ the same approach for loading registry/dde: try static extension, try [load] using ::reglib or ::ddelib provided via tcltest -load, finally try [package require].
- The pkgIndex.tcl files for registry and dde will only modify ::reglib or ::ddelib if the variable exists. If the test falls back to a [package require] this communicates the actual lib back to the test script for use in slave interps (meaning no modifications to the test design).
- registry-6.21 gets 'reg' constraint.
Variant committed to core-8-5-branch: The setting of
$::ddelib can be moved from pkgIndex.tcl to
winDde.test, this prevent test-specific code
to be installed.
I agree with putting the [load {} dde] first: this
is the situation that dde is compiled in static,
and it doesn't need the "package require"
machinery first. So, anyway, whether it works
completely or not, it's an improvement.
Please test in your environment.
I even wonder whether the ::tcltest::loadTestedCommands
machinery is still necessary: [load {} dde] works in the
build environment, [package require dde] works in the
installed environment, what more do we need?
Thanks!
Gentlemen, while you're busy pushing the envelope of 'make test', you might find 3549454 worth the effort ;)
Much simpler solution committed to core-8-5-branch:
In makefile.vc/Makefile.in, use the "package ifneeded"
command for both dde and reg, indicating where the
dll is. Then from the script, simply doing
::tcltest::loadTestedCommands
package require registry
works always: From the build directory, the dll
registrered by the "package ifneeded" will be
picked up without searching directories.
Only for winDde.test, we need to do a little
thing extra: extract ::ddelib from the ifneeded
script, so the remaining of the tests can use it.
Tested with both Makefile.in and makefile.vc
>Much simpler solution committed to core-8-5-branch:
Actually, this is twylite's suggestion (2) from 2012-07-27 06:46:42 PDT,
which turns out not to be that difficult at all.
Now merged to trunk as well.
Does this solve everything, or are there still things left to do?
>May be partially related to #3545367 'DDE test failures' *
I'm still getting those failures when using nmake, so it
looks unrelated after all.
No, this does not solve everything.
- Checkin [9792ff8b65] breaks 86 tests in info.test.
- Checkin [334aea1d5d] will break windde.test the moment the version of the DDE package changes.
- fCmd.test was missed in [334aea1d5d], so a test in fCmd gets skipped in the install environment.
- winDde-7.5 now fails in the install environment; I haven't investigated further, since in my opinion this is a bug introduced by an incorrect fix.
- The filesystem, msgcat and safe issues are still unresolved.
- Checkin [9792ff8b65] makes makefile.in and makefile.vc behave differently w.r.t. testing. Please explain.
You _cannot_ figure out ::ddelib in windde.test after a 'package require'. The logic requires intimate knowledge of the package version and the exact syntax of the 'ifneeded' script. Code locality demands that setting ::ddelib goes into pkgIndex.tcl.
The change to rely on 'package require' is also fragile. If specified with the exact version then multiple test files _must_ change every time the versions of dde or registry are updated. If specified without the exact version then the wrong DLL could be tested if there is for some reason a newer version provided via the pkgIndex in TCL_LIBRARY.
The _right_ way to do this is not to muck with the logic for the build environment, which has been working fine the whole time, and ensures that the built DLL is the one that gets tested. The logic that works is to try the static extension first, then try to load from a file explicitly indicated by the build system. What is missing is a fallback case for the install environment: if the former two fail, then try a package require and have it communicate to the test the location of the library actually loaded.
I am attaching td-3549770-3.patch that fixes these problems (except filesystem, msgcat, safe). Please don't make me fix them a forth time.
Patch to [6c2af8c78b] (trunk) for dde/reg tests and info.test regression
td-3549770-3.patch uploaded; apply to trunk (checkin [6c2af8c78b]).
>No, this does not solve everything.
I know, but it's a start
>- Checkin [9792ff8b65] breaks 86 tests in info.test.
Already fixed
> You _cannot_ figure out ::ddelib in windde.test after a 'package require'.
Sure that can be done, using "info loaded". We know the version, so
in practice I don't see the problem. Still working on that.
The way I see is that the "package" information storage
(using the "package ifneeded" stuff) keeps track what needs
to be done to load the package, with the possible version
numbers. It's better to use that, than some external
variable.
The external variable ensures that the correct DLL gets tested. Not some other DLL that matches the criteria.
This ddelib/reglib logic has been in place since 2005. It does the job. There is nothing wrong with it. And unlike 'package require' it is guaranteed to always test the right file.
> We know the version, so in practice I don't see the problem. Still working on that.
No, the test does not know the version of tclreg.dll or tcldde.dll, unless the test uses -exact and gets updated every time either of these versions changes. Which will not happen, because these are windows-specific, and no-one making the version changes is regularly running tests on windows (if they were, I wouldn't be filing bugs for problems introduced 9+ months ago).