Skip to content
Open
Show file tree
Hide file tree
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 Jun 6, 2025
3bc9ee3
Updated actions for ctest
oddbookworm Jun 6, 2025
578478b
Unity now a wrap
oddbookworm Jun 6, 2025
fac3c87
Deleted failed debian build
oddbookworm Jun 6, 2025
4d81bd2
Messed with ctest build to try to get it to work on CI... failed
oddbookworm Jun 7, 2025
943eafe
CI updates
oddbookworm Jun 7, 2025
e0b8ed7
.gitignore subprojects directory
oddbookworm Jun 7, 2025
a991b4a
ctest is now python package
oddbookworm Jun 7, 2025
96e35c0
Finished proper ctest support in ubuntu-checks action
oddbookworm Jun 7, 2025
7922bcf
Formatting
oddbookworm Jun 7, 2025
99ad425
Workflow no longer tries to upload nonexistent artifact and intention…
oddbookworm Jun 7, 2025
d383916
Properly reference matrix vars
oddbookworm Jun 7, 2025
e51672d
Maybe fixed cppcheck diff
oddbookworm Jun 7, 2025
50f3474
Split off declarations of base module to header and exposed everythin…
oddbookworm Jun 7, 2025
9724c02
Split off base declarations to base.h, and remove static linkage for …
oddbookworm Jun 8, 2025
4d058a1
Code cleanup, reverted unnecessary changes
oddbookworm Jun 9, 2025
4172f57
Code formatting
oddbookworm Jun 9, 2025
f8447fe
Make slightly less clever use of token pasting to make gcc happy
oddbookworm Jun 9, 2025
f0d6031
Removed accidentally added action
oddbookworm Aug 3, 2025
f1b8f43
Format
oddbookworm Aug 3, 2025
917b360
Removed extraneous beginning of file newline
oddbookworm Aug 7, 2025
ad295f2
Merge branch 'main' of https://github.com/pygame-community/pygame-ce …
oddbookworm Aug 14, 2025
5ddff7a
Fixed refcounting in ctests; Fixed issue where I was using the wrong …
oddbookworm Aug 14, 2025
ac84921
Addressed more review comments
oddbookworm Aug 14, 2025
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
5 changes: 3 additions & 2 deletions .github/workflows/run-ubuntu-checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# Also generates coverage information from unit tests. Note that for intrinsics,
# it only runs what gets compiled and would naturally run. It also is limited to
# what can run in a CI environment.

# Update this workflow when our min/max python minor versions update
# This workflow is necessary to ensure that we can build and run with
# a debug python build without too much worrying about SIGABRT being thrown
Expand All @@ -28,6 +29,7 @@ on:
# re-include current file to not be excluded
- '!.github/workflows/run-ubuntu-checks.yml'


pull_request:
branches: main
paths-ignore:
Expand Down Expand Up @@ -102,8 +104,7 @@ jobs:
id: build-pygame-ce
run: |
pyenv global ${{ matrix.python }}-debug
python dev.py build --lax --coverage --sanitize undefined

python dev.py build --lax --coverage --ctest --sanitize undefined
- name: Run tests
env:
SDL_VIDEODRIVER: "dummy"
Expand Down
4 changes: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@
# Ruff
.ruff_cache

# Meson subprojects
subprojects/*
!subprojects/*.wrap

# Other
envdev*
.virtualenv*
Expand Down
3 changes: 3 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ repos:
| ^.*\.svg$
| ^.*\.sfd$
| docs/LGPL.txt
| subprojects/.*
)$
- id: trailing-whitespace
exclude: |
Expand All @@ -23,6 +24,7 @@ repos:
| ^.*\.svg$
| ^.*\.sfd$
| docs/LGPL.txt
| subprojects/.*
)$

- repo: https://github.com/astral-sh/ruff-pre-commit
Expand All @@ -47,4 +49,5 @@ repos:
| src_c/include/sse2neon.h
| src_c/include/pythoncapi_compat.h
| src_c/pypm.c
| subprojects/.*
)$
175 changes: 175 additions & 0 deletions ctest/base_ctest.c
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;
}
13 changes: 13 additions & 0 deletions ctest/meson.build
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']
)
48 changes: 48 additions & 0 deletions ctest/test_common.h
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
16 changes: 13 additions & 3 deletions dev.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
]
COVERAGE_ARGS = ["-Csetup-args=-Dcoverage=true"]

CTEST_ARGS = ["-Csetup-args=-Dctest=true"]

# We assume this script works with any pip version above this.
PIP_MIN_VERSION = "23.1"

Expand Down Expand Up @@ -213,6 +215,7 @@ def cmd_build(self):
stripped = self.args.get("stripped", False)
sanitize = self.args.get("sanitize")
coverage = self.args.get("coverage", False)
ctest = self.args.get("ctest", False)
if wheel_dir and coverage:
pprint("Cannot pass --wheel and --coverage together", Colors.RED)
sys.exit(1)
Comment on lines +218 to 221
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 14, 2025

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:

         if wheel_dir and coverage:
             pprint("Cannot pass --wheel and --coverage together", Colors.RED)
             sys.exit(1)
+
+        if wheel_dir and ctest:
+            pprint("Cannot pass --wheel and --ctest together (test-only artifacts)", Colors.RED)
+            sys.exit(1)

Also applies to: 263-265

🤖 Prompt for AI Agents
In dev.py around lines 218-221 (and similarly at 263-265), the script currently
allows --ctest to affect wheel builds which can accidentally include test-only
modules; add a guard that checks if ctest is True and wheel_dir is set and then
print an error (use pprint with Colors.RED) and sys.exit(1) to disallow
combining --ctest with --wheel (mirror the existing coverage+wheels check) so
test artifacts are never packaged.

Copy link
Member Author

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Expand All @@ -226,6 +229,8 @@ def cmd_build(self):
build_suffix += "-sdl3"
if coverage:
build_suffix += "-cov"
if ctest:
build_suffix += "-ctest"
install_args = [
"--no-build-isolation",
f"-Cbuild-dir=.mesonpy-build{build_suffix}",
Expand Down Expand Up @@ -255,12 +260,14 @@ def cmd_build(self):
if coverage:
install_args.extend(COVERAGE_ARGS)

if ctest:
install_args.extend(CTEST_ARGS)

if sanitize:
install_args.append(f"-Csetup-args=-Db_sanitize={sanitize}")

info_str = (
f"with {debug=}, {lax=}, {sdl3=}, {stripped=}, {coverage=} and {sanitize=}"
)
info_str = f"with {debug=}, {lax=}, {sdl3=}, {stripped=}, {coverage=}, {ctest=}, and {sanitize=}"

if wheel_dir:
pprint(f"Building wheel at '{wheel_dir}' ({info_str})")
cmd_run(
Expand Down Expand Up @@ -416,6 +423,9 @@ def parse_args(self):
"supported if the underlying compiler supports the --coverage argument"
),
)
build_parser.add_argument(
"--ctest", action="store_true", help="Build the C-direct unit tests"
)

# Docs command
docs_parser = subparsers.add_parser("docs", help="Generate docs")
Expand Down
5 changes: 5 additions & 0 deletions meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -452,4 +452,9 @@ if not get_option('stripped')
subdir('buildconfig/stubs')
install_subdir('examples', install_dir: pg_dir, install_tag: 'pg-tag')
# TODO: install headers? not really important though

if get_option('ctest')
subproject('unity')
subdir('ctest')
endif
endif
10 changes: 7 additions & 3 deletions meson_options.txt
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,23 @@ option('midi', type: 'feature', value: 'enabled')
# Controls whether to make a "stripped" pygame install. Enabling this disables
# the bundling of docs/examples/tests/stubs in the wheels.
# The default behaviour is to bundle all of these.
option('stripped', type: 'boolean', value: 'false')
option('stripped', type: 'boolean', value: false)

# Controls whether to compile with -Werror (or its msvc equivalent). The default
# behaviour is to not do this by default
option('error_on_warns', type: 'boolean', value: 'false')
option('error_on_warns', type: 'boolean', value: false)

# Controls whether to error on build if generated docs are missing. Defaults to
# false.
option('error_docs_missing', type: 'boolean', value: 'false')
option('error_docs_missing', type: 'boolean', value: false)

# Controls whether to do a coverage build.
# This argument must be used together with the editable install.
option('coverage', type: 'boolean', value: false)

# Controls whether to do to a C unit test build. Defaults to false.
# If "stripped" is true, this is ignored.
option('ctest', type: 'boolean', value: false)

# Controls whether to use SDL3 instead of SDL2. The default is to use SDL2
option('sdl_api', type: 'integer', min: 2, max: 3, value: 2)
2 changes: 1 addition & 1 deletion src_c/base.c
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ static int pg_is_init = 0;
static bool pg_sdl_was_init = 0;
SDL_Window *pg_default_window = NULL;
pgSurfaceObject *pg_default_screen = NULL;
static int pg_env_blend_alpha_SDL2 = 0;
int pg_env_blend_alpha_SDL2 = 0;

/* compare compiled to linked, raise python error on incompatibility */
int
Expand Down
Loading
Loading