On 7 October 2010 14:35, Jacek Caban <jacek(a)codeweavers.com> wrote:
Hi Reece,
These tests need more thoughts. They should show how jscript exceptions are translated into IActiveScriptError. Your code doesn't provide mechanism to do so, so I would suggest to wait with sending patches that don't move us any forward until you have a clue how it should work. Otherwise you will have to deal with eventually bad decisions for no good reason.
Ok, so if I keep the current tests/ActiveScriptSite as they are and have something like: test_script_error(source, scode, description, lineno, charpos, linesource) { EXPECT CALL ActiveScriptSite_OnScriptError hres = parse_script_with_error(source); ok(hres == 0x8012345, ...) CHECK CALL ActiveScriptSite_OnScriptError ok(script_excep.scode == scode, ...) ok(script_excep.bstrDescription == description, ...) ok(script_lineno == lineno, ...) ok(script_charpos == charpos, ...) ok(script_linesource == linesource, ...) } test_script_errors() { test_script_error("var foo = new Object();\nfoo.bar;", 0x8012345, "Object does not support this property", 20, 11, "foo.bar;") test_script_error("new <>%^^;", 0x8012322, "Syntax error", 0, 0, "new <>%^^;") ... } where parse_script_with_error does the same thing as parse_script, but uses an ActiveScriptSite implementation that checks OnScriptError calls and stores the IActiveScriptError object for testing within test_script_error (like with my 3rd patch, but checking specific values). 1/ Does this look like a sensible approach to you? 2/ Do we care about the tests working on non-English locales (not sure how to get the above working on other languages -- specifically the description text)? 3/ Are we ok with making wine match the error description strings exactly, given the above tests? NOTE: 2 and 3 can be avoided by just checking the scode and checking that the description is set and non-NULL (as per my 3rd patch).
Also merging parsing_htmlscript is not a good idea. Its purpose is different than parse_script.
Ok, I'll keep them separate. - Reece