Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port GDScript test/debugging tools #41355

Merged
merged 2 commits into from
Sep 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion core/os/os.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,13 @@ class OS {

protected:
friend class Main;
// Needed by tests to setup command-line args.
friend int test_main(int argc, char *argv[]);

HasServerFeatureCallback has_server_feature_callback = nullptr;
RenderThreadMode _render_thread_mode = RENDER_THREAD_SAFE;

// functions used by main to initialize/deinitialize the OS
// Functions used by Main to initialize/deinitialize the OS.
void add_logger(Logger *p_logger);

virtual void initialize() = 0;
Expand Down
4 changes: 4 additions & 0 deletions modules/gdscript/SCsub
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,7 @@ if env["tools"]:
# Using a define in the disabled case, to avoid having an extra define
# in regular builds where all modules are enabled.
env_gdscript.Append(CPPDEFINES=["GDSCRIPT_NO_LSP"])

if env["tests"]:
env_gdscript.Append(CPPDEFINES=["TESTS_ENABLED"])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary? It should already be inherited from the main env which was cloned for env_gdscript no?

Copy link
Contributor Author

@Xrayez Xrayez Sep 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, because we're actually outside of the main/ env here. Recall #40726, this is what I meant when saying that some modules may eventually need this. Defining TESTS_ENABLED for all modules is also an option, but GDScript may be just an exception.

Technically, those are more like debug tools the way I see it, but they still qualify as something which facilitate manual testing process.

env_gdscript.add_source_files(env.modules_sources, "./tests/*.cpp")
28 changes: 28 additions & 0 deletions modules/gdscript/register_types.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@
#include "gdscript_cache.h"
#include "gdscript_tokenizer.h"

#ifdef TESTS_ENABLED
#include "tests/test_gdscript.h"
#include "tests/test_macros.h"
#endif

GDScriptLanguage *script_language_gd = nullptr;
Ref<ResourceFormatLoaderGDScript> resource_loader_gd;
Ref<ResourceFormatSaverGDScript> resource_saver_gd;
Expand Down Expand Up @@ -153,3 +158,26 @@ void unregister_gdscript_types() {
GDScriptParser::cleanup();
GDScriptAnalyzer::cleanup();
}

#ifdef TESTS_ENABLED
void test_tokenizer() {
TestGDScript::test(TestGDScript::TestType::TEST_TOKENIZER);
}

void test_parser() {
TestGDScript::test(TestGDScript::TestType::TEST_PARSER);
}

void test_compiler() {
TestGDScript::test(TestGDScript::TestType::TEST_COMPILER);
}

void test_bytecode() {
TestGDScript::test(TestGDScript::TestType::TEST_BYTECODE);
}

REGISTER_TEST_COMMAND("gdscript-tokenizer", &test_tokenizer);
REGISTER_TEST_COMMAND("gdscript-parser", &test_parser);
REGISTER_TEST_COMMAND("gdscript-compiler", &test_compiler);
REGISTER_TEST_COMMAND("gdscript-bytecode", &test_bytecode);
#endif
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,6 @@
#include "core/os/os.h"
#include "core/string_builder.h"

#include "modules/modules_enabled.gen.h"

#ifdef MODULE_GDSCRIPT_ENABLED

#include "modules/gdscript/gdscript_analyzer.h"
#include "modules/gdscript/gdscript_compiler.h"
#include "modules/gdscript/gdscript_parser.h"
Expand Down Expand Up @@ -183,21 +179,21 @@ static void test_compiler(const String &p_code, const String &p_script_path, con
}
}

MainLoop *test(TestType p_type) {
void test(TestType p_type) {
List<String> cmdlargs = OS::get_singleton()->get_cmdline_args();

if (cmdlargs.empty()) {
return nullptr;
return;
}

String test = cmdlargs.back()->get();
if (!test.ends_with(".gd")) {
print_line("This test expects a path to a GDScript file as its last parameter. Got: " + test);
return nullptr;
return;
}

FileAccessRef fa = FileAccess::open(test, FileAccess::READ);
ERR_FAIL_COND_V_MSG(!fa, nullptr, "Could not open file: " + test);
ERR_FAIL_COND_MSG(!fa, "Could not open file: " + test);

Vector<uint8_t> buf;
int flen = fa->get_len();
Expand Down Expand Up @@ -230,21 +226,6 @@ MainLoop *test(TestType p_type) {
case TEST_BYTECODE:
print_line("Not implemented.");
}

return nullptr;
}

} // namespace TestGDScript

#else

namespace TestGDScript {

MainLoop *test(TestType p_type) {
ERR_PRINT("The GDScript module is disabled, therefore GDScript tests cannot be used.");
return nullptr;
}

} // namespace TestGDScript

#endif
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@
#ifndef TEST_GDSCRIPT_H
#define TEST_GDSCRIPT_H

#include "core/os/main_loop.h"

namespace TestGDScript {

enum TestType {
Expand All @@ -42,7 +40,8 @@ enum TestType {
TEST_BYTECODE,
};

MainLoop *test(TestType p_type);
void test(TestType p_type);

} // namespace TestGDScript

#endif // TEST_GDSCRIPT_H
3 changes: 0 additions & 3 deletions tests/SCsub
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@ env.tests_sources = []

env_tests = env.Clone()

# Enable test framework and inform it of configuration method.
env_tests.Append(CPPDEFINES=["DOCTEST_CONFIG_IMPLEMENT"])

Comment on lines -9 to -11
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this and define DOCTEST_CONFIG_IMPLEMENT in test_macros.cpp once. Including doctest header twice in source files doesn't protect the implementation to be included once, and leads to linker errors with duplicate symbols.

# We must disable the THREAD_LOCAL entirely in doctest to prevent crashes on debugging
# Since we link with /MT thread_local is always expired when the header is used
# So the debugger crashes the engine and it causes weird errors
Expand Down
42 changes: 42 additions & 0 deletions tests/test_macros.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*************************************************************************/
/* test_macros.cpp */
/*************************************************************************/
/* This file is part of: */
/* GODOT ENGINE */
/* https://godotengine.org */
/*************************************************************************/
/* Copyright (c) 2007-2020 Juan Linietsky, Ariel Manzur. */
/* Copyright (c) 2014-2020 Godot Engine contributors (cf. AUTHORS.md). */
/* */
/* Permission is hereby granted, free of charge, to any person obtaining */
/* a copy of this software and associated documentation files (the */
/* "Software"), to deal in the Software without restriction, including */
/* without limitation the rights to use, copy, modify, merge, publish, */
/* distribute, sublicense, and/or sell copies of the Software, and to */
/* permit persons to whom the Software is furnished to do so, subject to */
/* the following conditions: */
/* */
/* The above copyright notice and this permission notice shall be */
/* included in all copies or substantial portions of the Software. */
/* */
/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, */
/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF */
/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.*/
/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY */
/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, */
/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE */
/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */
/*************************************************************************/

#define DOCTEST_CONFIG_IMPLEMENT
#include "test_macros.h"

Map<String, TestFunc> *test_commands = nullptr;

int register_test_command(String p_command, TestFunc p_function) {
if (!test_commands) {
test_commands = new Map<String, TestFunc>;
}
test_commands->insert(p_command, p_function);
return 0;
}
16 changes: 16 additions & 0 deletions tests/test_macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@
#ifndef TEST_MACROS_H
#define TEST_MACROS_H

#include "core/map.h"
#include "core/variant.h"

// See documentation for doctest at:
// https://github.com/onqtam/doctest/blob/master/doc/markdown/readme.md#reference
#include "thirdparty/doctest/doctest.h"
Expand Down Expand Up @@ -104,4 +107,17 @@ DOCTEST_STRINGIFY_VARIANT(PackedVector2Array);
DOCTEST_STRINGIFY_VARIANT(PackedVector3Array);
DOCTEST_STRINGIFY_VARIANT(PackedColorArray);

// Register test commands to be launched from the command-line.
// For instance: REGISTER_TEST_COMMAND("gdscript-parser" &test_parser_func).
// Example usage: `godot --test gdscript-parser`.

typedef void (*TestFunc)();
extern Map<String, TestFunc> *test_commands;
int register_test_command(String p_command, TestFunc p_function);

#define REGISTER_TEST_COMMAND(m_command, m_function) \
DOCTEST_GLOBAL_NO_WARNINGS(DOCTEST_ANONYMOUS(_DOCTEST_ANON_VAR_)) = \
register_test_command(m_command, m_function); \
DOCTEST_GLOBAL_NO_WARNINGS_END()

#endif // TEST_MACROS_H
50 changes: 36 additions & 14 deletions tests/test_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
#include "test_class_db.h"
#include "test_color.h"
#include "test_expression.h"
#include "test_gdscript.h"
#include "test_gradient.h"
#include "test_gui.h"
#include "test_math.h"
Expand All @@ -56,40 +55,63 @@
#include "tests/test_macros.h"

int test_main(int argc, char *argv[]) {
bool run_tests = true;

// Convert arguments to Godot's command-line.
List<String> args;

for (int i = 0; i < argc; i++) {
args.push_back(String::utf8(argv[i]));
}
OS::get_singleton()->set_cmdline("", args);
Copy link
Contributor Author

@Xrayez Xrayez Aug 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first argument is empty because there's no way to know the executable name with the existing testing interface, but I'm not sure whether that's essential in the first place.


// Run custom test tools.
if (test_commands) {
for (Map<String, TestFunc>::Element *E = test_commands->front(); E; E = E->next()) {
if (args.find(E->key())) {
const TestFunc &test_func = E->get();
test_func();
run_tests = false;
break;
}
}
if (!run_tests) {
delete test_commands;
return 0;
}
}
// Doctest runner.
doctest::Context test_context;
List<String> valid_arguments;
List<String> test_args;

// Clean arguments of "--test" from the args.
int argument_count = 0;
for (int x = 0; x < argc; x++) {
if (strncmp(argv[x], "--test", 6) != 0) {
valid_arguments.push_back(String(argv[x]));
argument_count++;
test_args.push_back(String(argv[x]));
}
}
// Convert Godot command line arguments back to standard arguments.
char **args = new char *[valid_arguments.size()];
for (int x = 0; x < valid_arguments.size(); x++) {
char **doctest_args = new char *[test_args.size()];
for (int x = 0; x < test_args.size(); x++) {
// Operation to convert Godot string to non wchar string.
CharString cs = valid_arguments[x].utf8();
CharString cs = test_args[x].utf8();
const char *str = cs.get_data();
// Allocate the string copy.
args[x] = new char[strlen(str) + 1];
doctest_args[x] = new char[strlen(str) + 1];
// Copy this into memory.
std::memcpy(args[x], str, strlen(str) + 1);
memcpy(doctest_args[x], str, strlen(str) + 1);
}

test_context.applyCommandLine(valid_arguments.size(), args);
test_context.applyCommandLine(test_args.size(), doctest_args);

test_context.setOption("order-by", "name");
test_context.setOption("abort-after", 5);
test_context.setOption("no-breaks", true);

for (int x = 0; x < valid_arguments.size(); x++) {
delete[] args[x];
for (int x = 0; x < test_args.size(); x++) {
delete[] doctest_args[x];
}
delete[] args;
delete[] doctest_args;

return test_context.run();
}