Unit tests for Object creation and property getter/setter #45411

a-ivanov · 2021-01-24T14:09:47Z

More Object tests as was noted in #43440.

Xrayez · 2021-01-26T13:14:46Z

Debugging on Windows gives me crash with the following backtrace:

ntdll.dll!00007ffb602b8fe2() (Unknown Source:0)
ntdll.dll!00007ffb602c1412() (Unknown Source:0)
ntdll.dll!00007ffb602c171a() (Unknown Source:0)
ntdll.dll!00007ffb602ca6d9() (Unknown Source:0)
ntdll.dll!00007ffb6020135d() (Unknown Source:0)
ntdll.dll!00007ffb602006e1() (Unknown Source:0)
godot.windows.tools.64.exe!_free_base(void * block) Line 105 (c:\Users\Xrayez\AppData\Local\Programs\Microsoft VS Code\minkernel\crts\ucrt\src\appcrt\heap\free_base.cpp:105)
godot.windows.tools.64.exe!Memory::free_static(void * p_ptr, bool p_pad_align) Line 170 (core\os\memory.cpp:170)
godot.windows.tools.64.exe!memdelete<ScriptInstance>(ScriptInstance * p_class) Line 119 (core\os\memory.h:119)
godot.windows.tools.64.exe!Object::~Object() Line 1776 (core\object\object.cpp:1776)
godot.windows.tools.64.exe!TestObject::_DOCTEST_ANON_FUNC_1791() Line 219 (tests\test_object.h:219)
godot.windows.tools.64.exe!doctest::Context::run() Line 6168 (thirdparty\doctest\doctest.h:6168)
godot.windows.tools.64.exe!test_main(int argc, char * * argv) Line 130 (tests\test_main.cpp:130)
godot.windows.tools.64.exe!Main::test_entrypoint(int argc, char * * argv, bool & tests_need_run) Line 469 (main\main.cpp:469)
godot.windows.tools.64.exe!widechar_main(int argc, wchar_t * * argv) Line 149 (platform\windows\godot_windows.cpp:149)
godot.windows.tools.64.exe!_main() Line 185 (platform\windows\godot_windows.cpp:185)
godot.windows.tools.64.exe!main(int argc, char * * argv) Line 199 (platform\windows\godot_windows.cpp:199)
[Inline Frame] godot.windows.tools.64.exe!invoke_main() Line 78 (d:\agent\_work\63\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:78)
godot.windows.tools.64.exe!__scrt_common_main_seh() Line 288 (d:\agent\_work\63\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
kernel32.dll!00007ffb60027c24() (Unknown Source:0)
ntdll.dll!00007ffb6022d4d1() (Unknown Source:0)

You'll also want to run the binary with --test --no-breaks so the debugger won't break on ClassDB tests currently.

Xrayez · 2021-01-26T13:21:29Z

tests/test_object.h

+	_MockScriptInstance script_instance;
+	object.set_script_instance(&script_instance);


This seems to resolve the crash above (in this particular test case):

Suggested change

_MockScriptInstance script_instance;

object.set_script_instance(&script_instance);

_MockScriptInstance *script_instance = memnew(_MockScriptInstance);

object.set_script_instance(script_instance);

Script instance will be freed automatically by the object's destructor, which actually caused the crash.

Generally, you'll also likely want to instantiate Object with memnew as well for consistency with the rest of the codebase (just like Nodes), but don't forget to memdelete those objects if you do.

The same applies to other test cases.

Xrayez · 2021-01-26T13:21:53Z

tests/test_object.h

+	CHECK(valid);
+	Variant actual_value;
+	CHECK_MESSAGE(
+			script_instance.get("some_name", actual_value),


Once the above is done:

Suggested change

script_instance.get("some_name", actual_value),

script_instance->get("some_name", actual_value),

a-ivanov · 2021-01-28T13:02:02Z

Thanks @Xrayez! I will definitely fix these, a little busy ATM.

a-ivanov · 2021-01-28T22:45:06Z

Debugging on Windows gives me crash with the following backtrace:

ntdll.dll!00007ffb602b8fe2() (Unknown Source:0)
ntdll.dll!00007ffb602c1412() (Unknown Source:0)
ntdll.dll!00007ffb602c171a() (Unknown Source:0)
ntdll.dll!00007ffb602ca6d9() (Unknown Source:0)
ntdll.dll!00007ffb6020135d() (Unknown Source:0)
ntdll.dll!00007ffb602006e1() (Unknown Source:0)
godot.windows.tools.64.exe!_free_base(void * block) Line 105 (c:\Users\Xrayez\AppData\Local\Programs\Microsoft VS Code\minkernel\crts\ucrt\src\appcrt\heap\free_base.cpp:105)
godot.windows.tools.64.exe!Memory::free_static(void * p_ptr, bool p_pad_align) Line 170 (core\os\memory.cpp:170)
godot.windows.tools.64.exe!memdelete<ScriptInstance>(ScriptInstance * p_class) Line 119 (core\os\memory.h:119)
godot.windows.tools.64.exe!Object::~Object() Line 1776 (core\object\object.cpp:1776)
godot.windows.tools.64.exe!TestObject::_DOCTEST_ANON_FUNC_1791() Line 219 (tests\test_object.h:219)
godot.windows.tools.64.exe!doctest::Context::run() Line 6168 (thirdparty\doctest\doctest.h:6168)
godot.windows.tools.64.exe!test_main(int argc, char * * argv) Line 130 (tests\test_main.cpp:130)
godot.windows.tools.64.exe!Main::test_entrypoint(int argc, char * * argv, bool & tests_need_run) Line 469 (main\main.cpp:469)
godot.windows.tools.64.exe!widechar_main(int argc, wchar_t * * argv) Line 149 (platform\windows\godot_windows.cpp:149)
godot.windows.tools.64.exe!_main() Line 185 (platform\windows\godot_windows.cpp:185)
godot.windows.tools.64.exe!main(int argc, char * * argv) Line 199 (platform\windows\godot_windows.cpp:199)
[Inline Frame] godot.windows.tools.64.exe!invoke_main() Line 78 (d:\agent\_work\63\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:78)
godot.windows.tools.64.exe!__scrt_common_main_seh() Line 288 (d:\agent\_work\63\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
kernel32.dll!00007ffb60027c24() (Unknown Source:0)
ntdll.dll!00007ffb6022d4d1() (Unknown Source:0)

You'll also want to run the binary with --test --no-breaks so the debugger won't break on ClassDB tests currently.

Interesting, I am running the test on Windows 10 (project compiled with MSVC) and it finishes successfully.

Xrayez · 2021-01-28T23:41:25Z

tests/test_object.h

+}
+
+TEST_CASE("[Object] Built-in property setter") {
+	ClassDB::register_class<_TestDerivedObject>();


Just wanted to note that ClassDB is used globally throughout tests, but as long as it doesn't affect other tests, this should be fine I think.

Xrayez · 2021-01-28T23:43:54Z

Interesting, I am running the test on Windows 10 (project compiled with MSVC) and it finishes successfully.

It depends on how you run tests, a Godot project may certainly display a backtrace upon crash (given debug symbols are enabled). If I run tests without debugger, then I see no apparent crash either.

If you look at Linux Builds / Editor with sanitizers checks, then you'd see various memory leaks and access violations which a debugger might not catch as well.

Xrayez

Should be good to merge!

akien-mga · 2021-01-28T23:58:58Z

Thanks!

Calinou added enhancement topic:tests labels Jan 24, 2021

Calinou added this to the 4.0 milestone Jan 24, 2021

akien-mga requested a review from a team January 25, 2021 23:00

Xrayez suggested changes Jan 26, 2021

View reviewed changes

Unit tests for Object creation and property getter/setter

db0f3da

a-ivanov force-pushed the more-object-tests branch from 12f8e75 to db0f3da Compare January 28, 2021 22:57

a-ivanov requested a review from Xrayez January 28, 2021 23:11

Xrayez reviewed Jan 28, 2021

View reviewed changes

Xrayez approved these changes Jan 28, 2021

View reviewed changes

akien-mga approved these changes Jan 28, 2021

View reviewed changes

akien-mga merged commit fbb4742 into godotengine:master Jan 28, 2021

akien-mga mentioned this pull request Jul 4, 2022

More object unit tests #44387

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unit tests for Object creation and property getter/setter #45411

Unit tests for Object creation and property getter/setter #45411

a-ivanov commented Jan 24, 2021

Xrayez commented Jan 26, 2021

Xrayez Jan 26, 2021 •

edited

Loading

Xrayez Jan 26, 2021

a-ivanov commented Jan 28, 2021

a-ivanov commented Jan 28, 2021 •

edited

Loading

Xrayez Jan 28, 2021

Xrayez commented Jan 28, 2021 •

edited

Loading

Xrayez left a comment

akien-mga commented Jan 28, 2021

		_MockScriptInstance script_instance;
		object.set_script_instance(&script_instance);

	script_instance.get("some_name", actual_value),
	script_instance->get("some_name", actual_value),

Unit tests for Object creation and property getter/setter #45411

Unit tests for Object creation and property getter/setter #45411

Conversation

a-ivanov commented Jan 24, 2021

Xrayez commented Jan 26, 2021

Xrayez Jan 26, 2021 • edited Loading

Choose a reason for hiding this comment

Xrayez Jan 26, 2021

Choose a reason for hiding this comment

a-ivanov commented Jan 28, 2021

a-ivanov commented Jan 28, 2021 • edited Loading

Xrayez Jan 28, 2021

Choose a reason for hiding this comment

Xrayez commented Jan 28, 2021 • edited Loading

Xrayez left a comment

Choose a reason for hiding this comment

akien-mga commented Jan 28, 2021

Xrayez Jan 26, 2021 •

edited

Loading

a-ivanov commented Jan 28, 2021 •

edited

Loading

Xrayez commented Jan 28, 2021 •

edited

Loading