Tcl Source Code

* 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

>Without _msgcat_ (in-build situation)
Of course, I meant:
Without registry (in-build situation)

msgcat-0.7 is fixed now: If the registry package is present, msgcat is able
to determine in a more complex way what the real locale is.
Just skip msgcat-0.7 in that case. Without msgcat (in-build situation)
the test will be executed normally.

One to go......

I still get the msgcat-0.7 and safe-14.1 failures when running outside the build tree (and not when running an in-build 'nmake test').  I don't really know where to start investigating the msgcat issue, and I haven't had a chance to dig into the safe issue.

Good fix! Thanks!

Does this mean that this issue can be closed now?

filsystem-7.1.1 and 7.1.2 tests were being skipped due to the loaddll constraint being 0. 

Committed a fix to trunk [d3bb61ba] that sets loaddll if the packages load, and corrects the path used in 7.1.1.

Thanks! committed

File Added - 450149: td-3549770-4.patch

Hi.  The changes to filesystem.test in [c1dc55ad] fail in both build and install environments:

- filesystem-7.1.1 in build: couldn't load library "simplefs:/./Release_VC10/tcldde14.dll": no such file or directory
- filesystem-7.1.1 in install: can't read "::ddelib": no such variable

Cause #1: no [package require] in .test, so [package versions] returns empty and subsequent logic generates an error, resulting in ddelib/reglib not being set.
Cause #2: simplefs is rooted in [pwd], and both tests do 'cd [file dirname [info nameof]]', so the library must be relative to the Tcl shell.

Proposed solution: apply the following diff (also attached as td-3549770-4.patch with CRLF eol):


--- tests\fileSystem.test
+++ tests\fileSystem.test
@@ -17,17 +17,19 @@
        file delete -force link.file
        file delete -force dir.link
        file delete -force [file join dir.dir linkinside.file]
     }

+testConstraint loaddll 0
 catch {
     ::tcltest::loadTestedCommands
     package require -exact Tcltest [info patchlevel]
-    set ::ddever [lindex [lsort [package versions dde]] end]
+    set ::ddever [package require dde]
     set ::ddelib [lindex [package ifneeded dde $::ddever] 1]
-    set ::regver [lindex [lsort [package versions registry]] end]
+    set ::regver [package require registry 1.3.0]
     set ::reglib [lindex [package ifneeded registry $::regver] 1]
+    testConstraint loaddll 1
 }

 # Test for commands defined in Tcltest executable
 testConstraint testfilesystem              [llength [info commands ::testfilesystem]]
 testConstraint testsetplatform             [llength [info commands ::testsetplatform]]
@@ -508,31 +510,31 @@
     while {![catch {testfilesystem 0}]} {}
 }

 test filesystem-7.1.1 {load from vfs} -setup {
     set dir [pwd]
-} -constraints {win testsimplefilesystem} -body {
+} -constraints {win testsimplefilesystem loaddll} -body {
     # This may cause a crash on exit
-    cd [file dirname [info nameof]]
+    cd [file dirname $::ddelib]
     testsimplefilesystem 1
     # This loads dde via a complex copy-to-temp operation
-    load simplefs:/$::ddelib dde
+    load simplefs:/[file tail $::ddelib] dde
     testsimplefilesystem 0
     return ok
     # The real result of this test is what happens when Tcl exits.
 } -cleanup {
     cd $dir
 } -result ok
 test filesystem-7.1.2 {load from vfs, and then unload again} -setup {
     set dir [pwd]
-} -constraints {win testsimplefilesystem} -body {
+} -constraints {win testsimplefilesystem loaddll} -body {
     # This may cause a crash on exit
-    cd [file dirname [info nameof]]
+    cd [file dirname $::reglib]
     testsimplefilesystem 1
     # This loads reg via a complex copy-to-temp operation
-    load simplefs:/$::reglib Registry
-    unload simplefs:/$::reglib
+    load simplefs:/[file tail $::reglib] Registry
+    unload simplefs:/[file tail $::reglib]
     testsimplefilesystem 0
     return ok
     # The real result of this test is what happens when Tcl exits.
 } -cleanup {
     cd $dir

Fix for [3545363] is now in branch bug-3545363.  Should merge cleanly onto trunk (other than ChangeLog).

Will test filesystem-* shortly.  I don't yet understand the causes of msgcat-0.7 and safe-14.1 so no suggestions at this point.

> These tests are redundant - if the [package require -exact] fails then the
> reg/dde constraint will be 0 and these tests will never execute.
> Conversely if the [package require -exact] passes then the dde/reg
> constrained tests will execute but we're already guaranteed of the correct
> DLL version.
You are right about that. Inproved now, such that the tests accept
higher dll versions as well. Good point! Thanks!
> And should Tcl behave according to expectations in the install environment?
> If so, how do we verify that?
That's why I agree that this is a good feature.

At the moment, various win32 tests still hang, due to
Bug [3545363], but as soon as that's fixed I will
run all win32 again.

Are the msgcat-0.7 and safe-14.1 tests the only
remaining failures, when run in the install environment?
I'm open to suggestions how to fix that.

> Tcl never promised to be able to run the testcases in the installed environment, so in fact this should be considered a feature, not a bug.

And should Tcl behave according to expectations in the install environment?  If so, how do we verify that?

> So, what's wrong with the following reasoning.

It works, but it's brittle.  There are now 4+ places that the reg/dde version numbers must be kept in sync.  Tcl builds with newer versions of reg/dde will necessarily fail against older versions of the test suite (which generally shouldn't happen in regression testing).  Still, it works, and it's your call on what fix gets accepted.

> Yes that's the way to assure the right version is loaded. 

The test should not care that the 'right version' is loaded.  The test should care only that a version with the minimum required compatibility is loaded.  Using -exact makes the test unnecessarily specific and incurs additional work for future maintainers.  In short, it's brittle.

The test execution environment should ensure that the DLL intended for testing is the one actually loaded; in the build environment this requires some additional logic to communicate the DLL location to the .test file.

> I now added an additional test-case winDde-1.0 and registry-1.0, which check whether the right dll is tested. If not, this test will fail but all others will run normally.

These tests are redundant - if the [package require -exact] fails then the reg/dde constraint will be 0 and these tests will never execute.  Conversely if the [package require -exact] passes then the dde/reg constrained tests will execute but we're already guaranteed of the correct DLL version.

> filesystem-7.1.[12] should pass now from outside the build environment.

Thanks, will test shortly.

filesystem-7.1.[12] should pass now from outside the build environment.

data_type - 360894

So, what's wrong with the following reasoning.

In Makefile/makefile.vc we use the "-load" or "-loadfile" option
to specify which versions we want to test:
package ifneeded dde 1.4.0b1 [list load [file normalize ${DDE_DLL_FILE}] dde]
package ifneeded registry 1.3.0 [list load [file normalize ${REG_DLL_FILE}] registry]
In the test case, we execute ::tcltest::loadTestedCommands
immediately before the [package require]. This way we are sure that the
mentioned versions match exactly the dll's we want to test. So,
using -exact, we are assured we are testing the right dll.

The big advantage of this is that testcases can always
use "package require" to load the right dll. The -load/-loadfile
options can control exactly which dll is being loaded by
the test. If you don't specify -load/-loadfile, then the
normal "package require" machinery will run.

> unless the test uses -exact and gets updated every time
>either of these versions changes
Yes that's the way to assure the right version is loaded. Maybe
the win32 tests are not run as often as they should, but
before every release they are all tested. People get
more active once a new release is approaching.

I now added an additional test-case winDde-1.0 and
registry-1.0, which check whether the right dll is tested.
If not, this test will fail but all others will run normally.

Tcl never promised to be able to run the testcases
in the installed environment, so in fact this should
be considered a feature, not a bug. Therefore
changing the group to 8.6b3.

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).

>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.

td-3549770-3.patch uploaded; apply to trunk (checkin [6c2af8c78b]).

File Added - 450126: td-3549770-3.patch

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.

>May be partially related to #3545367 'DDE test failures' *
I'm still getting those failures when using nmake, so it
looks unrelated after all.

>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?

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

Gentlemen, while you're busy pushing the envelope of 'make test', you might find 3549454 worth the effort ;)

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!

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.

File Added - 450018: td-3549770-3.patch

Sounds good

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.

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.

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.

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.

File Added - 449954: td-3549770-1.patch

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
 }] {

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.

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.