Last modified: 2013-11-15 11:09:55 UTC
Following up on [1] [1] http://www.mediawiki.org/wiki/Extension_talk:Scribunto#Register_external_library_using_registerInterface.28.29_25635 ## Report Using the following registration method $this->getEngine()->registerInterface( dirname( __FILE__ ) . '/' . 'smw.library.lua', $lib, array() ); will cause an internal error Lua file does not exist: C:\xampp\htdocs\mw\extensions\Scribunto\engines\LuaCommon/lualib/C:\xampp\htdocs\mw\extensions\SemanticMediaWiki\includes\lua/smw.library.lua Verification ## Environment MediaWiki 1.21alpha (37875db), Scribunto (a852755) ## Anomie responded The problem is the Windows directory paths with drive letter; since c98cc645 Scribunto has supported absolute Unix-style paths in registerInterface(), and 8fd45026 tried to extend this to Windows but failed to consider drive letters.
Could we also make sure that test cases don't have to same problem. For example: return parent::getTestModules() + array( 'PropertyLibraryTests' => __DIR__ . '/PropertyLibraryTests.lua', ); Right now when trying to load a PropertyLibraryTests.lua from a non-Scribunto directory it returns with a Invalid argument supplied for foreach() in C:\xampp\htdocs\mw\extensions\Scribunto\tests\engines\LuaCommon\LuaEngineTestBase.php on line 183 Unexpected non-MediaWiki exception encountered, of type "Exception" exception 'Exception' with message 'Failed to load module PropertyLibraryTests' But PropertyLibraryTests.lua do exists in folder C:\xampp\htdocs\mw\extensions\SemanticMediaWiki\tests\phpunit\includes\lua which is the same directory that holds PropertyLibraryTest.php Because as long as tests do not work, [1] is blocked. [1] https://gerrit.wikimedia.org/r/#/c/56393/
(In reply to comment #1) > Could we also make sure that test cases don't have to same problem. It doesn't appear that they do, as the code in question only accepts absolute paths and does not try to prepend anything. > Invalid argument supplied for foreach() in > C: > \xampp\htdocs\mw\extensions\Scribunto\tests\engines\LuaCommon\LuaEngineTestBa > se.php > on line 183 That warning seems to indicate that your getTestModules() is not in fact returning an array. > Because as long as tests do not work, [1] is blocked. > > [1] https://gerrit.wikimedia.org/r/#/c/56393/ I see no unit tests being added in that changeset.
> > Because as long as tests do not work, [1] is blocked. > > > > [1] https://gerrit.wikimedia.org/r/#/c/56393/ > > I see no unit tests being added in that changeset. Well, I updated the change but as you can see their are two issues first PropertyLibraryTests.lua returns locally with Notice: unserialize(): Error at offset 114 of 118 bytes in C:\xampp\htdocs\mw\extensions\Scribunto\engines\LuaStandalone\LuaStandaloneEngine.php on line 384 Lua error: Internal error: Unable to decode message. Backtrace: #0 but the more severe issue is '''Jenkins''' since Scribunto is not a dependency "$wgAutoloadClasses['Scribunto_LuaEngineTestBase']" or class_exists() won't work because Jenkins checks the directory (-- extensions/SemanticMediaWiki) and not the "UnitTestsList hook" so doing " ... extends Scribunto_LuaEngineTestBase" will fail in Jenkins for extensions. It is not really a nice design for a test case rather I would expect to invoke the Scribunto_LuaEngineTestBase class and set properties via setter/getter but I can't do this because the $moduleName is protected (of course I could try going through the ReflectionClass but this shouldn't be necessary).
(In reply to comment #3) > > > Because as long as tests do not work, [1] is blocked. > > > > > > [1] https://gerrit.wikimedia.org/r/#/c/56393/ > > > > I see no unit tests being added in that changeset. > > Well, I updated the change but as you can see their are two issues first > PropertyLibraryTests.lua returns locally with > > Notice: unserialize(): Error at offset 114 of 118 bytes in > C: > \xampp\htdocs\mw\extensions\Scribunto\engines\LuaStandalone\LuaStandaloneEngi > ne.php > on line 384 Lua error: Internal error: Unable to decode message. Backtrace: > #0 I wonder if that's bug 46294. > but the more severe issue is '''Jenkins''' since Scribunto is not a > dependency > "$wgAutoloadClasses['Scribunto_LuaEngineTestBase']" or class_exists() won't > work because Jenkins checks the directory (-- extensions/SemanticMediaWiki) > and > not the "UnitTestsList hook" so doing > > " ... extends Scribunto_LuaEngineTestBase" will fail in Jenkins for > extensions. I tried to ask hashar about this a while ago. Now that we have an actual example of the problem, perhaps he can tell us what to do about this situation. > It is not really a nice design for a test case You're welcome to try to redesign it. Note that all the tests must be run against both engines, the tests should actually run as separate tests and not one test with hundreds of assertions, it should not require every test class to be updated should another engine be added, and it's best not to require a ton of boilerplate to do so. Good luck.
> I wonder if that's bug 46294. Just tested [1] and PHPUnit didn't crash with unserialize(), tests return with OK, but incomplete or skipped tests! Tests: 3, Assertions: 2, Skipped: 1. of course only after I copied the smw.property.lua file into the LuaCommon\lualib folder. [1] https://gerrit.wikimedia.org/r/#/c/56425/
Back the original problem of registerInterface() which generates a concatenated string of C:\xampp\htdocs\mw\extensions\Scribunto\engines\LuaCommon/lualib/C:\xampp\htdocs\mw\extensions\SemanticMediaWiki\includes\lua/smw.property.lua The problem lies with the Scribunto_LuaEngine class where the check $fileName[0] !== DIRECTORY_SEPARATOR will not provide sufficient clarity and end up putting $this->getLuaLibDir() {C:\xampp\htdocs\mw\extensions\Scribunto\engines\LuaCommon/lualib/} and $fileName {C:\xampp\htdocs\mw\extensions\SemanticMediaWiki\includes\lua/smw.property.lua} together. protected function normalizeModuleFileName( $fileName ) { return $fileName[0] !== DIRECTORY_SEPARATOR ? "{$this->getLuaLibDir()}/{$fileName}" : $fileName; }
Please fill a new bug for the Jenkins issue reported in Comment #3: --- but the more severe issue is '''Jenkins''' since Scribunto is not a dependency "$wgAutoloadClasses['Scribunto_LuaEngineTestBase']" or class_exists() won't work because Jenkins checks the directory (-- extensions/SemanticMediaWiki) and not the "UnitTestsList hook" so doing --- I suspect the request is to add Scribunto has an extension dependency to the SemanticMediaWiki extension.
(In reply to comment #6) > Back the original problem of registerInterface() which generates a > concatenated > string of > C:\xampp\htdocs\mw\extensions\Scribunto\engines\LuaCommon/lualib/C: > \xampp\htdocs\mw\extensions\SemanticMediaWiki\includes\lua/smw.property.lua > > The problem lies with the Scribunto_LuaEngine class where the check > $fileName[0] !== DIRECTORY_SEPARATOR will not provide sufficient clarity and As I said, it's not recognizing that the path beginning with the drive letter is absolute, so it's treating it as relative. Gerrit change #56596 should theoretically fix it.
(In reply to comment #7) > Please fill a new bug for the Jenkins issue reported in Comment #3: > > I suspect the request is to add Scribunto has an extension dependency to the > SemanticMediaWiki extension. I'm just to lazy for creating yet another bug, I went ahead and added the dependency [1]. [1] https://gerrit.wikimedia.org/r/#/c/56570/
(In reply to comment #8) > (In reply to comment #6) > > Gerrit change #56596 should theoretically fix it. Gerrit change #56596 returns with Warning: preg_match(): No ending matching delimiter '>' found in C:\xampp\htdocs\mw\extensions\Scribunto\engines\LuaCommon\LuaCommon.php on line 142
Changeset merged to fix the original issue here. Unit test file loading seems fine as well, and the Jenkins dependency has been merged too.
Change 95396 had a related patch set uploaded by MarkAHershberger: (bug 46635) Recognize Windows path+drive letter https://gerrit.wikimedia.org/r/95396
Change 95396 merged by MarkAHershberger: (bug 46635) Recognize Windows path+drive letter https://gerrit.wikimedia.org/r/95396
No open patches to review here (backport patch to REL_1_21 got merged), hence resetting status to RESOLVED FIXED. Backport_to_Stable flag might be set to "+" by hexmode.
