GNOME Bugzilla – Bug 779871
jsUnit: Explicitly check if tempTop.parent is defined
Last modified: 2017-03-11 19:42:23 UTC
On Ubuntu's CI machines we were seeing libsecret's tests failing like this: FAIL: libsecret/test-js-lookup.js Gjs-Message: JS WARNING: [resource:///org/gnome/gjs/modules/jsUnit.js 54]: reference to undefined property tempTop.parent FAIL: libsecret/test-js-clear.js Gjs-Message: JS WARNING: [resource:///org/gnome/gjs/modules/jsUnit.js 54]: reference to undefined property tempTop.parent FAIL: libsecret/test-js-store.js Gjs-Message: JS WARNING: [resource:///org/gnome/gjs/modules/jsUnit.js 54]: reference to undefined property tempTop.parent I poked around and it turns out that this is one way to explicitly check for a variable being defined. This fixes these failing tests. What do you think? I don't really understand what the code does (it's marked as a hack), so review appreciated.
Created attachment 347660 [details] [review] jsUnit: Explicitly check if tempTop.parent is defined This avoid 'reference to undefined property' warnings.
Review of attachment 347660 [details] [review]: Yeah, I don't really know what this code does either, sadly. JSUnit was copied in from the JSUnit project [1] because it was probably the only JS unit testing suite in 2008 when GJS was written that worked outside a browser. GJS itself doesn't use it anymore. I didn't realize anybody still used it, so I didn't bother to fix the undefined property warnings in it! I recommend that libsecret use some more convenient package for unit testing. JSUnit tests are really painful to write and that's one reason I created jasmine-gjs [2], just to include a shameless plug. But I'd be happy to include this patch. Feel free to commit it with the change from != to !==. By the way, odd that SpiderMonkey does not recognize "while(obj.maybeUndefinedProp)" as checking for undefined, whereas "if(obj.maybeUndefinedProp)" is considered OK :-/ [1] http://www.jsunit.net/ [2] https://github.com/ptomato/jasmine-gjs ::: modules/jsUnit.js @@ +52,3 @@ if (!tempTop) { tempTop = window; + while (typeof tempTop.parent != 'undefined') { Best practice is to use !== everywhere.
Thnks - fixed and pushed. I'm not a member of libsecret - just came across the warning when looking at it in the distro. Not sure what the acutal maintainers think about jsUnit. :) Attachment 347660 [details] pushed as a6f229e - jsUnit: Explicitly check if tempTop.parent is defined