[Bug] Unit Test Failures on Win7
Reported by Thell Fowler | October 8th, 2009 @ 03:33 PM | in v1.0.0-rc2
There seems to be a difference causing test failures when using Win7 shlwapi.dll 6.1.7100. These tests pass fine using shlwapi.dll 6.00.2900.5512
2>[ PASSED ] 124 tests.
2>[ FAILED ] 12 tests, listed below:
2>[ FAILED ] PathAppendTest.ParentBasedBasicPathAndBasicName
2>[ FAILED ] PathAppendTest.ParentBasedFullPathAndBasicName
2>[ FAILED ] PathAppendTest.ParentBasedBasicPathAndRootPath
2>[ FAILED ] PathAppendTest.ParentBasedFullPathAndRootPath
2>[ FAILED ] PathAppendTest.ParentOfTwoDeepPathBackOneParentAndBasicName
2>[ FAILED ] PathRemoveFileSpecTest.BasicPathNoFileDoubleMiddleSlashWithoutEndSlash
2>[ FAILED ] PathCanonicalizeTest.ParentOverflow
2>[ FAILED ] PathCanonicalizeTest.ParentParentOverflow
2>[ FAILED ] PathCanonicalizeTest.SameParentOverflow
2>[ FAILED ] PathCanonicalizeTest.SameParentParentOverflow
2>[ FAILED ] PathCanonicalizeTest.ParentSameOverflow
2>[ FAILED ] PathCanonicalizeTest.ParentSameParentOverflow
2>12 FAILED TESTS
Comments and changes to this ticket
-
Thell Fowler October 8th, 2009 @ 03:33 PM
A msdn forum thread has been opened on this issue at:
http://social.msdn.microsoft.com/Forums/en-US/vcprerelease/thread/0...
-
Thell Fowler October 8th, 2009 @ 03:35 PM
- Milestone set to v1.0.0-rc1
-
Jocelyn Legault October 8th, 2009 @ 10:26 PM
- Title changed from Unit Test Failures on Win7 to [Bug] Unit Test Failures on Win7
-
Thell Fowler October 16th, 2009 @ 06:51 PM
- Milestone changed from v1.0.0-rc1 to v1.0.0
-
Thell Fowler October 16th, 2009 @ 06:58 PM
- Milestone changed from v1.0.0 to v1.0.0-rc2
-
Dave Brotherstone October 18th, 2009 @ 09:07 PM
- State changed from new to open
For the PathRemoveFileSpec failure, that is simply an improvement on their algorithm:
They're now handling c:\abc\def in PathRemoveFileSpec more sensibly, to become c:\abc rather than c:\abc\For PathAppend, this is mainly due to a very strange change in behaviour in Win7, to do with PathCanonicalize.
..\abc\def = abc\def
- explicitly NOT what the docs say, it should be \abc\def, but, following the logic that if it starts with ..\, the \ is removed, we accept that change and continue...
\..\abc\def = \abc\def
- ok, this is the same as the previous version
..\abc\..\def = \def
- Pardon? ..\abc\def = abc\def, but ..\abc\..\def = \def, what on earth?
- if we re-wrote the path to be ..\def, which would be the same directory (relatively), it would return 'def', so a .. in the middle causes that rule to break
..\abc\def\..\ghi = abc\ghi
- but alas, that isn't always the case!
Hence, this is the logic that must be mirrored:
..\abc\def abc\def ..\abc\..\def \def ..\abc\def\..\ghi abc\ghi
The generic_string version of PathCanonicalize needs to be re-developed using a list of path-sections, and then it can process the path from the start instead of the end, which will make the algorithm significantly simpler, albeit less efficient.
However, as can be seen from this table, the logic of whether the leading backslash is present is very odd. -
Jocelyn Legault October 18th, 2009 @ 09:31 PM
..\abc..\def = \def That makes sense IMO
if ..\abc becomes abc and ..\abc\def = \abc\def, then [..\abc][..\def] =>
[..\def] and [..\def] => \def -
Dave Brotherstone October 18th, 2009 @ 10:12 PM
Logic identified, I think, as if the "root" is ever touched, ignoring any leading .., then a backslash is prepended to the output.
..\abc\..\def
- ignore initial ..
- abc, now in directory abc
- .., now at root, prepend \ to output
- def, now at \def.
- output result - \def
Requesting a vote on this:
- Copy the generic_string's into TCHARs, call the win32 function
and copy back
or - Reimplement and do the windows version check for the edge case
- ignore initial ..
-
Jocelyn Legault October 18th, 2009 @ 11:10 PM
My vote goes strongly in favor of 1 for the following reasons:
- We will be assured that we will have an identical behaviour as
the original Win32 one, no matter which platform / compiler we
use.
- This function is never used in tight loops where realtime performance is needed.
So, while it'd be nice to have a "native" std::string implementation, it
think the better solution is the simplest in this case. - We will be assured that we will have an identical behaviour as
the original Win32 one, no matter which platform / compiler we
use.
-
Dave Brotherstone October 20th, 2009 @ 07:52 PM
- State changed from open to needs_ack
- Assigned user changed from Dave Brotherstone to npp-community
(from [250a8d32fb4d613b02b5fca91826ef25bc663097]) Common.cpp: PathXXX functions on Windows 7 fix
- PathCanonicalize has been made to simply call the Win32 function, as the difference between Win7 and prior versions is too great
- PathRemoveFileSpec added some extra logic in for double backslashes in the middle of a path
- Additional debug output added to tests to aid in debugging tests if they fail
[#4 state:needs_ack responsible:npp-community]
Signed-off-by: Dave Brotherstone davegb@pobox.com
http://github.com/davegb3/npp-community/commit/250a8d32fb4d613b02b5... -
Thell Fowler October 20th, 2009 @ 08:10 PM
- State changed from needs_ack to acked
Ack. Will get on pu soon.
-
npp-community October 21st, 2009 @ 05:19 PM
- State changed from acked to needs_ack
(from [2a4b25270681344fb1162d0b0a8452b61e429007]) Common.cpp: PathXXX functions on Windows 7 fix
- PathCanonicalize has been made to simply call the Win32 function, as the difference between Win7 and prior versions is too great
- PathRemoveFileSpec added some extra logic in for double backslashes in the middle of a path
- Additional debug output added to tests to aid in debugging tests if they fail
[#4 state:needs_ack responsible:npp-community]
Signed-off-by: Dave Brotherstone davegb@pobox.com
http://github.com/npp-community/npp-community/commit/2a4b2527068134... -
Thell Fowler October 21st, 2009 @ 05:19 PM
- State changed from needs_ack to proposed
(from [5abf9740311f688a2a1e3d5369615fe60f30955f]) Merge branch 'db/LH-4/PathFuncFix' into pu
- db/LH-4/PathFuncFix: Common.cpp: PathXXX functions on Windows 7 fix
[#4 state:proposed]
Signed-off-by: Thell Fowler git@tbfowler.name
http://github.com/npp-community/npp-community/commit/5abf9740311f68... -
Thell Fowler October 21st, 2009 @ 05:37 PM
Thanks Dave!
I can confirm Win7 now passes all tests, I haven't tried this build on XP yet to verify it still passes all tests.
-
Thell Fowler October 26th, 2009 @ 04:46 PM
- State changed from proposed to candidate
(from [67d226aa8999e6cb2db9fc618912af20ca2d57d2]) Merge branch 'db/LH-4/fix-win7-path-handling' into next
- db/LH-4/fix-win7-path-handling: Common.cpp: PathXXX functions on Windows 7 fix
[#4 state:candidate]
Signed-off-by: Thell Fowler git@tbfowler.name
http://github.com/npp-community/npp-community/commit/67d226aa8999e6... -
Thell Fowler November 29th, 2009 @ 06:34 AM
- State changed from candidate to resolved
(from [71c6fddd0f5457ad1dcbf834914ccf52b7ac4c43]) Merge branch 'db/LH-4/fix-win7-path-handling'
- db/LH-4/fix-win7-path-handling: Common.cpp: PathXXX functions on Windows 7 fix
[#4 state:resolved]
Signed-off-by: Thell Fowler git@tbfowler.name
http://github.com/npp-community/npp-community/commit/71c6fddd0f5457...
Please Sign in or create a free account to add a new ticket.
With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป
Notepad++ Community Release
People watching this ticket
Tags
Referenced by
- 4 [Bug] Unit Test Failures on Win7 [#4 state:resolved]
- 4 [Bug] Unit Test Failures on Win7 [#4 state:needs_ack responsible:npp-community]
- 4 [Bug] Unit Test Failures on Win7 [#4 state:needs_ack responsible:npp-community]
- 4 [Bug] Unit Test Failures on Win7 [#4 state:proposed]
- 4 [Bug] Unit Test Failures on Win7 [#4 state:candidate]