tobijk / caius

A functional testing framework in object-oriented Tcl
MIT License
26 stars 5 forks source link

Unable to get setup_before to be called #4

Closed apnadkarni closed 9 years ago

apnadkarni commented 9 years ago

I have a simple test -

itcl::class FibonacciTests {
    inherit Testing::TestObject

    method setup_before {} {
        foreach subdir {stylesheets js images} {
            file copy -force -- [::woof::test::source_path samples public $subdir] [::woof::test::target_path public]
        }
    }

    method test-1 {} {}
}

exit [[FibonacciTests #auto] run $::argv]

Running the above, setup_before does not get called. Looking at the code for TestObj::setup and TestObj::teardown, I'm wondering at the following line:

foreach {class} [lreverse [$this info heritage]] {

The above loop would not include the leaf test class itself, would it? Changing the line to

foreach {class} [concat [$this info class] [lreverse [$this info heritage]]] {

fixed the problem. However, I'm not very familiar with Itcl either so perhaps I'm declaring my methods wrong?

/Ashok

tobijk commented 9 years ago

Actually, according to the documentation, it should include the leaf test class:

objName info heritage Returns the current class name and the entire list of base classes in the order that they are traversed for member lookup and object destruction.

And on my system your code is working without modifications. Can we have run into an Itcl bug?

apnadkarni commented 9 years ago

Quite possible. What version of Itcl do you have? I'm using 4.0.2

tobijk commented 9 years ago

I'm using the old Itcl 3. I checked here:

https://github.com/tcltk/itcl/blob/master/doc/class.n

and the description of the info heritage command has not changed, so I assume this is a bug in the version 4 series, but we will need a work-around, I'm afraid.

apnadkarni commented 9 years ago

OK, found what the issue is with Itcl 4

package require Itcl
itcl::class B {
   public method hierarchy {} { return [$this info heritage] }
}
itcl::class C {
   inherit B
   public method hierarchy {} { return [$this info heritage] }
}
itcl::class D {
   inherit B
}

Then,

(tcl) 49 % C #auto
c0
(tcl) 51 % c0 hierarchy
::C ::B
(tcl) 52 % D #auto
d0
(tcl) 53 % d0 hierarchy
::B

So clearly, info heritage seems to find its class type based on where it's called from. I don't know if this is a bug or a feature. I have logged a bug against Itcl. Not hopeful of a quick fix as itcl support is rather sporadic these days.

apnadkarni commented 9 years ago

Also found that calling from the global scope works. For example [d0 info hierarchy] in the above example works correctly. I'm going to try the workaround of using [uplevel #0 $this info heritage] instead of [$this info heritage].

tobijk commented 9 years ago

Without uplevel, can you try what happens, if you first fully qualify $this using itcl::code and/or namespace which?

If we do uplevel #0 I think we will have to always use namespace which, as well, for it to work namespace-agnostic. Not sure, I'm still learning when it comes to these nuances in Tcl.

apnadkarni commented 9 years ago

namespace which does not work (same effect as existing code). I got some other errors using itcl::code, most likely because I'm not really familiar with ITcl.

I landed up with [uplevel #0 [list $this] info inherit]

Do you want me to make that change and a pull request? Or make the change yourself? Or just leave it in my fork for now?

tobijk commented 9 years ago

You came up with the solution, please make a merge request : ) Why is the [list $this] necessary? And you meant to use info heritage, I assume?

apnadkarni commented 9 years ago

Yes, info heritage, not info inherit.

The [list] is in case the object name contains spaces, it needs to be protected as single "token" for uplevel. I don't know if Itcl permits spaces in names or not. TclOO does.

I'll make a pull request

/Ashok

apnadkarni commented 9 years ago

By the way [namespace which] is not needed because the generated name for $this is fully qualified.

tobijk commented 9 years ago

Thanks a lot for figuring this out. Closing.