-
-
Notifications
You must be signed in to change notification settings - Fork 201
C UNIT TESTING!!! (this time on the right repo) #3477
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
Open
oddbookworm
wants to merge
24
commits into
main
Choose a base branch
from
andrew-c-unit-testing
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
7f1684f
Beginnings of c unit testing with base module (with necessary changes…
oddbookworm 3bc9ee3
Updated actions for ctest
oddbookworm 578478b
Unity now a wrap
oddbookworm fac3c87
Deleted failed debian build
oddbookworm 4d81bd2
Messed with ctest build to try to get it to work on CI... failed
oddbookworm 943eafe
CI updates
oddbookworm e0b8ed7
.gitignore subprojects directory
oddbookworm a991b4a
ctest is now python package
oddbookworm 96e35c0
Finished proper ctest support in ubuntu-checks action
oddbookworm 7922bcf
Formatting
oddbookworm 99ad425
Workflow no longer tries to upload nonexistent artifact and intention…
oddbookworm d383916
Properly reference matrix vars
oddbookworm e51672d
Maybe fixed cppcheck diff
oddbookworm 50f3474
Split off declarations of base module to header and exposed everythin…
oddbookworm 9724c02
Split off base declarations to base.h, and remove static linkage for …
oddbookworm 4d058a1
Code cleanup, reverted unnecessary changes
oddbookworm 4172f57
Code formatting
oddbookworm f8447fe
Make slightly less clever use of token pasting to make gcc happy
oddbookworm f0d6031
Removed accidentally added action
oddbookworm f1b8f43
Format
oddbookworm 917b360
Removed extraneous beginning of file newline
oddbookworm ad295f2
Merge branch 'main' of https://github.com/pygame-community/pygame-ce …
oddbookworm 5ddff7a
Fixed refcounting in ctests; Fixed issue where I was using the wrong …
oddbookworm ac84921
Addressed more review comments
oddbookworm File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,175 @@ | ||
#include <Python.h> | ||
|
||
#include "base.h" | ||
#include "test_common.h" | ||
|
||
/* setUp and tearDown must be nonstatic void(void) */ | ||
void setUp(void) {} | ||
|
||
void tearDown(void) {} | ||
|
||
/** | ||
* @brief Tests _pg_is_int_tuple when passed a tuple of ints | ||
*/ | ||
PG_CTEST(test__pg_is_int_tuple_nominal)(PyObject *self, PyObject *_null) { | ||
PyObject *arg1 = Py_BuildValue("(iii)", 1, 2, 3); | ||
if (!arg1) { | ||
// exception already set by Py_BuildValue | ||
return NULL; | ||
} | ||
|
||
PyObject *arg2 = Py_BuildValue("(iii)", -1, -2, -3); | ||
if (!arg2) { | ||
// exception already set by Py_BuildValue | ||
return NULL; | ||
} | ||
|
||
PyObject *arg3 = Py_BuildValue("(iii)", 1, -2, -3); | ||
if (!arg3) { | ||
// exception already set by Py_BuildValue | ||
return NULL; | ||
} | ||
|
||
TEST_ASSERT_EQUAL(1, _pg_is_int_tuple(arg1)); | ||
TEST_ASSERT_EQUAL(1, _pg_is_int_tuple(arg2)); | ||
TEST_ASSERT_EQUAL(1, _pg_is_int_tuple(arg3)); | ||
|
||
Py_DECREF(arg1); | ||
Py_DECREF(arg2); | ||
Py_DECREF(arg3); | ||
|
||
Py_RETURN_NONE; | ||
} | ||
|
||
/** | ||
* @brief Tests _pg_is_int_tuple when passed a tuple of non-numeric values | ||
*/ | ||
PG_CTEST(test__pg_is_int_tuple_failureModes)(PyObject *self, PyObject *_null) { | ||
PyObject *arg1 = | ||
Py_BuildValue("(sss)", (char *)"Larry", (char *)"Moe", (char *)"Curly"); | ||
if (!arg1) { | ||
// exception already set by Py_BuildValue | ||
return NULL; | ||
} | ||
|
||
PyObject *arg2 = Py_BuildValue("(zzz)", NULL, NULL, NULL); // tuple of None's | ||
if (!arg2) { | ||
// exception already set by Py_BuildValue | ||
return NULL; | ||
} | ||
|
||
PyObject *arg3 = Py_BuildValue("(OOO)", arg1, arg2, arg1); | ||
if (!arg3) { | ||
// exception already set by Py_BuildValue | ||
return NULL; | ||
} | ||
|
||
TEST_ASSERT_EQUAL(0, _pg_is_int_tuple(arg1)); | ||
TEST_ASSERT_EQUAL(0, _pg_is_int_tuple(arg2)); | ||
TEST_ASSERT_EQUAL(0, _pg_is_int_tuple(arg3)); | ||
|
||
Py_DECREF(arg1); | ||
Py_DECREF(arg2); | ||
Py_DECREF(arg3); | ||
|
||
Py_RETURN_NONE; | ||
} | ||
|
||
/** | ||
* @brief Tests _pg_is_int_tuple when passed a tuple of floats | ||
*/ | ||
PG_CTEST(test__pg_is_int_tuple_floats)(PyObject *self, PyObject *_null) { | ||
PyObject *arg1 = Py_BuildValue("(ddd)", 1.0, 2.0, 3.0); | ||
if (!arg1) { | ||
// exception already set by Py_BuildValue | ||
return NULL; | ||
} | ||
|
||
PyObject *arg2 = Py_BuildValue("(ddd)", -1.1, -2.2, -3.3); | ||
if (!arg2) { | ||
// exception already set by Py_BuildValue | ||
return NULL; | ||
} | ||
|
||
PyObject *arg3 = Py_BuildValue("(ddd)", 1.0, -2.0, -3.1); | ||
if (!arg3) { | ||
// exception already set by Py_BuildValue | ||
return NULL; | ||
} | ||
|
||
TEST_ASSERT_EQUAL(0, _pg_is_int_tuple(arg1)); | ||
TEST_ASSERT_EQUAL(0, _pg_is_int_tuple(arg2)); | ||
TEST_ASSERT_EQUAL(0, _pg_is_int_tuple(arg3)); | ||
|
||
Py_DECREF(arg1); | ||
Py_DECREF(arg2); | ||
Py_DECREF(arg3); | ||
|
||
Py_RETURN_NONE; | ||
} | ||
|
||
/*=======Test Reset Option=====*/ | ||
/* This must be void(void) */ | ||
void resetTest(void) { | ||
tearDown(); | ||
setUp(); | ||
} | ||
|
||
/*=======Exposed Test Reset Option=====*/ | ||
static PyObject *reset_test(PyObject *self, PyObject *_null) { | ||
resetTest(); | ||
|
||
Py_RETURN_NONE; | ||
} | ||
|
||
/*=======Run The Tests=======*/ | ||
static PyObject *run_tests(PyObject *self, PyObject *_null) { | ||
UnityBegin("base_ctest.c"); | ||
// This macro has calls to setUp and tearDown already baked into it | ||
// so there's no need to explicitly call resetTest between test cases | ||
RUN_TEST_PG_INTERNAL(test__pg_is_int_tuple_nominal); | ||
RUN_TEST_PG_INTERNAL(test__pg_is_int_tuple_failureModes); | ||
RUN_TEST_PG_INTERNAL(test__pg_is_int_tuple_floats); | ||
|
||
return PyLong_FromLong(UnityEnd()); | ||
} | ||
|
||
static PyMethodDef base_test_methods[] = { | ||
{"test__pg_is_int_tuple_nominal", | ||
(PyCFunction)test__pg_is_int_tuple_nominal, METH_NOARGS, | ||
"Tests _pg_is_int_tuple when passed a tuple of ints"}, | ||
{"test__pg_is_int_tuple_failureModes", | ||
(PyCFunction)test__pg_is_int_tuple_failureModes, METH_NOARGS, | ||
"Tests _pg_is_int_tuple when passed a tuple of non-numeric values"}, | ||
{"test__pg_is_int_tuple_floats", (PyCFunction)test__pg_is_int_tuple_floats, | ||
METH_NOARGS, "Tests _pg_is_int_tuple when passed a tuple of floats"}, | ||
{"reset_test", (PyCFunction)reset_test, METH_NOARGS, | ||
"Explicitly runs tearDown(); setUp(). Note: RUN_TEST_PG_INTERNAL calls " | ||
"setUp/tearDown around each test; run_tests does not call reset_test " | ||
"explicitly."}, | ||
{"run_tests", (PyCFunction)run_tests, METH_NOARGS, | ||
"Runs all the tests in this test suite"}, | ||
{NULL, NULL, 0, NULL}}; | ||
|
||
MODINIT_DEFINE(base_ctest) { | ||
PyObject *module; | ||
|
||
static struct PyModuleDef _module = { | ||
PyModuleDef_HEAD_INIT, | ||
"base_ctest", | ||
"C unit tests for the pygame.base internal implementation", | ||
-1, | ||
base_test_methods, | ||
NULL, | ||
NULL, | ||
NULL, | ||
NULL}; | ||
|
||
/* create the module */ | ||
module = PyModule_Create(&_module); | ||
if (!module) { | ||
return NULL; | ||
} | ||
|
||
return module; | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
unity_subproject = subproject('unity') | ||
unity_dependency = unity_subproject.get_variable('unity_dep') | ||
|
||
base_ctest = py.extension_module( | ||
'base_ctest', | ||
'base_ctest.c', | ||
c_args: warnings_error, | ||
dependencies: [pg_base_deps, unity_dependency], | ||
sources: ['../src_c/base.c'], | ||
install: true, | ||
subdir: pg, | ||
include_directories: ['../src_c'] | ||
) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
#include <Python.h> | ||
|
||
#include "unity.h" | ||
|
||
#ifndef TEST_COMMON_H | ||
#define TEST_COMMON_H | ||
|
||
struct TestCase { | ||
char *test_name; | ||
int line_num; | ||
}; | ||
|
||
/* | ||
This will take some explanation... the PG_CTEST macro defines two things | ||
for an individual test case. The test case itself, and a struct instance | ||
called meta_TEST_CASE_NAME. The struct has two pieces of important | ||
information that unity needs: the name in string format and the line | ||
number of the test. This would be an absolute nighmare to maintain by | ||
hand, so I defined a macro to do it automagically for us. | ||
|
||
The RUN_TEST_PG_INTERNAL macro then references that struct for each test | ||
case that we tell it about and automatically populates the unity fields | ||
with the requisite data. | ||
|
||
Note that the arguments to the test function must be *exactly* | ||
(PyObject * self, PyObject * _null), but due to gcc throwing a fit, I | ||
cannot just use token pasting to have the macro generate that part for me | ||
*/ | ||
#define PG_CTEST(TestFunc) \ | ||
static struct TestCase meta_##TestFunc = {#TestFunc, __LINE__}; \ | ||
static PyObject *TestFunc | ||
|
||
#define RUN_TEST_PG_INTERNAL(TestFunc) \ | ||
{ \ | ||
Unity.CurrentTestName = meta_##TestFunc.test_name; \ | ||
Unity.CurrentTestLineNumber = meta_##TestFunc.line_num; \ | ||
Unity.NumberOfTests++; \ | ||
if (TEST_PROTECT()) { \ | ||
setUp(); \ | ||
TestFunc(self, _null); \ | ||
} \ | ||
if (TEST_PROTECT()) { \ | ||
tearDown(); \ | ||
} \ | ||
UnityConcludeTest(); \ | ||
} | ||
|
||
#endif // #ifndef TEST_COMMON_H |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Avoid shipping C-test artifacts in wheels by guarding
--ctest
+--wheel
.Right now
--ctest
also affects wheel builds, which risks packaging test-only modules into distributable artifacts. Recommend disallowing or warning on--wheel
with--ctest
to prevent accidental publication.Proposed guard near the existing coverage check:
Also applies to: 263-265
🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless @ankith26 overrides me, I want to leave this as-is. It's only included in one CI run, which we don't publish wheels from anyway. However, we might want to occasionally share wheels as developers with the ctest suite enabled
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.