Menu
▾
▴
[Tcl-bugs] [ tcl-Feature Requests-3549770 ] Multiple test failures running tcltest outside build tree
|
From: SourceForge.net <no...@so...> - 2012-07-30 12:17:45
|
Feature Requests item #3549770, was opened at 2012-07-27 01:45 Message generated for change (Comment added) made by twylite You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=360894&aid=3549770&group_id=10894 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: 53. Configure and Build Tools Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Twylite (twylite) Assigned to: Jan Nijtmans (nijtmans) Summary: Multiple test failures running tcltest outside build tree Initial Comment: * 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 ---------------------------------------------------------------------- >Comment By: Twylite (twylite) Date: 2012-07-30 05:17 Message: > 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. ---------------------------------------------------------------------- Comment By: Jan Nijtmans (nijtmans) Date: 2012-07-30 04:44 Message: filesystem-7.1.[12] should pass now from outside the build environment. ---------------------------------------------------------------------- Comment By: Jan Nijtmans (nijtmans) Date: 2012-07-30 03:43 Message: 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. ---------------------------------------------------------------------- Comment By: Twylite (twylite) Date: 2012-07-30 03:12 Message: 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). ---------------------------------------------------------------------- Comment By: Jan Nijtmans (nijtmans) Date: 2012-07-30 02:54 Message: >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. ---------------------------------------------------------------------- Comment By: Twylite (twylite) Date: 2012-07-30 02:34 Message: td-3549770-3.patch uploaded; apply to trunk (checkin [6c2af8c78b]). ---------------------------------------------------------------------- Comment By: Twylite (twylite) Date: 2012-07-30 02:32 Message: 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. ---------------------------------------------------------------------- Comment By: Jan Nijtmans (nijtmans) Date: 2012-07-28 16:28 Message: >May be partially related to #3545367 'DDE test failures' * I'm still getting those failures when using nmake, so it looks unrelated after all. ---------------------------------------------------------------------- Comment By: Jan Nijtmans (nijtmans) Date: 2012-07-28 16:27 Message: >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? ---------------------------------------------------------------------- Comment By: Jan Nijtmans (nijtmans) Date: 2012-07-28 15:55 Message: 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 ---------------------------------------------------------------------- Comment By: Alexandre Ferrieux (ferrieux) Date: 2012-07-28 10:07 Message: Gentlemen, while you're busy pushing the envelope of 'make test', you might find 3549454 worth the effort ;) ---------------------------------------------------------------------- Comment By: Jan Nijtmans (nijtmans) Date: 2012-07-28 07:56 Message: 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! ---------------------------------------------------------------------- Comment By: Twylite (twylite) Date: 2012-07-28 06:28 Message: 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. ---------------------------------------------------------------------- Comment By: Jan Nijtmans (nijtmans) Date: 2012-07-27 07:43 Message: Sounds good ---------------------------------------------------------------------- Comment By: Twylite (twylite) Date: 2012-07-27 07:34 Message: 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. ---------------------------------------------------------------------- Comment By: Twylite (twylite) Date: 2012-07-27 07:23 Message: 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. ---------------------------------------------------------------------- Comment By: Twylite (twylite) Date: 2012-07-27 06:46 Message: 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. ---------------------------------------------------------------------- Comment By: Jan Nijtmans (nijtmans) Date: 2012-07-27 05:17 Message: 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. ---------------------------------------------------------------------- Comment By: Twylite (twylite) Date: 2012-07-27 04:38 Message: 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 }] { ---------------------------------------------------------------------- Comment By: Twylite (twylite) Date: 2012-07-27 03:54 Message: 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. ---------------------------------------------------------------------- Comment By: Twylite (twylite) Date: 2012-07-27 02:36 Message: 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. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=360894&aid=3549770&group_id=10894 |