[Bug 12083] New: ok() test doesn't support nested calls

http://bugs.winehq.org/show_bug.cgi?id=12083

Summary: ok() test doesn't support nested calls
           Product: Wine
           Version: CVS/GIT
          Platform: All
        OS/Version: other
            Status: UNCONFIRMED
          Severity: minor
          Priority: P4
         Component: testcases
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]

When ok() (wine/test.h) is used recursively, the reported filename and line
numbers don't reflect the location in the code where the test actually failed.
For example, in dlls/kernel32/tests/change.c, the
ok(FinishNotificationThread(thread), ...) calls will report a line number
inside FinishNotificationThread(), where ok() is also used. This is because the
tls_data structure stores only a single filename and line number value, and
every call to ok() updates them to the "current" values.

Is it erroneous for test code to recursively call ok() (a quick test indicates
that this happens hundreds of times throughout Wine's tests), or should ok()
chain these values to handle recursive calls?

[Bug 12083] ok() test doesn't support nested calls

http://bugs.winehq.org/show_bug.cgi?id=12083

--- Comment #1 from Dmitry Timoshkov   2008-03-17 21:54:47 ---
ok(FinishNotificationThread(thread), "Missed notification\n");

should be replaced with

ret = FinishNotificationThread(thread);
ok(ret, "Missed notification\n");

Please send a patch.

[Bug 12083] ok() test doesn't support nested calls

http://bugs.winehq.org/show_bug.cgi?id=12083

--- Comment #2 from Michael Moss   2008-03-17 22:10:54 ---
Does this mean that nested ok() calls are not allowed? I wasn't worried
specifically about that FinishNotificationThread() call, but just giving it as
an example. Unfortunately, it looks like there are hundreds of such nested
calls in the code (a quick test gave me ~1500, and that didn't even finish
because some of the tests crashed before they could all run). Do all such
instances need to be changed? If so, should there also be a recursion
check/assert to winetest_set_location() and winetest_ok()?

[Bug 12083] ok() test doesn't support nested calls

http://bugs.winehq.org/show_bug.cgi?id=12083

Dmitry Timoshkov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1

--- Comment #3 from Dmitry Timoshkov   2008-03-17 23:43:13 ---
(In reply to comment #2)
> Does this mean that nested ok() calls are not allowed?

It was not designed for recursive calls, so, yes, all the instances
that cause recursive ok() calls should be fixed.
> I wasn't worried
> specifically about that FinishNotificationThread() call, but just giving it as
> an example. Unfortunately, it looks like there are hundreds of such nested
> calls in the code (a quick test gave me ~1500, and that didn't even finish
> because some of the tests crashed before they could all run). Do all such
> instances need to be changed? If so, should there also be a recursion
> check/assert to winetest_set_location() and winetest_ok()?

It's not worth it IMO.

[Bug 12083] ok() test doesn't support nested calls

http://bugs.winehq.org/show_bug.cgi?id=12083

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|testcases                   |-unknown

[Bug 12083] ok() test doesn't support nested calls

http://bugs.winehq.org/show_bug.cgi?id=12083

--- Comment #4 from Austin English   2008-10-30 02:16:11 ---
Is this still an issue in current (1.1.7 or newer) wine?

[Bug 12083] ok() test doesn't support nested calls

http://bugs.winehq.org/show_bug.cgi?id=12083

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|CVS/GIT                     |unspecified

--- Comment #5 from Austin English   2009-01-19 15:14:29 ---
Removing deprecated CVS/GIT version tag. Please retest in current git. If the
bug is still present in today's wine, but was not present in some earlier
version of wine, please update version field to earliest known version of wine
that had the bug. Thanks!

[Bug 12083] ok() test doesn't support nested calls

http://bugs.winehq.org/show_bug.cgi?id=12083

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |ABANDONED

--- Comment #6 from Austin English   2009-07-21 14:49:06 ---
Abandoned. If you still have a problem in current (1.1.26 or newer) wine, and
can provide the needed information, feel free to reopen.

[Bug 12083] ok() test doesn't support nested calls

http://bugs.winehq.org/show_bug.cgi?id=12083

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #7 from Austin English   2009-07-21 15:10:11 ---
Closing.

[Bug 12083] ok() test doesn't support nested calls

http://bugs.winehq.org/show_bug.cgi?id=12083

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Platform|All                         |Other

--- Comment #8 from Austin English  2012-02-23 15:22:54 CST ---
Removing deprecated 'All' Platform.