Skip to content

test: Run integration tests across Cinema 4D versions (2024,2025,2026) - #513

Closed
karthikbekalp wants to merge 3 commits into
mainlinefrom
feature/ci-tests
Closed

test: Run integration tests across Cinema 4D versions (2024,2025,2026)#513
karthikbekalp wants to merge 3 commits into
mainlinefrom
feature/ci-tests

Conversation

@karthikbekalp

Copy link
Copy Markdown
Contributor

What was the problem/requirement? (What/Why)

We need to run integration tests on all the versions of Cinema 4d that we currently support and not just 2026.

What was the solution? (How)

Updated the tests to run on all the versions that we support i.e. 2024 to 2026.

We still have issues with Redshift tests failing because of a Maxon bug so marked those tests as XFAIL so that even if it flakes it does not break the workflow.

What is the impact of this change?

We have higher coverage and make sure our changes are backwards compatible.

How was this change tested?

Ran the tests here: https://github.com/aws-deadline/deadline-cloud-for-cinema-4d/actions/runs/31415004267/job/93541820105

Was this change documented?

No documentation changes required. Only test changes.

Is this a breaking change?

No.


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Signed-off-by: Karthik Bekal Pattathana <133984042+karthikbekalp@users.noreply.github.com>
@karthikbekalp
karthikbekalp requested a review from a team as a code owner August 10, 2026 18:25
@github-actions github-actions Bot added the waiting-on-maintainers Waiting on the maintainers to review. label Aug 10, 2026
@karthikbekalp
karthikbekalp enabled auto-merge (squash) August 10, 2026 18:26
@karthikbekalp

Copy link
Copy Markdown
Contributor Author

Resolving this in favor of : #515

auto-merge was automatically disabled August 10, 2026 18:43

Pull request was closed

inputs:
c4d_version:
description: Cinema 4D major version to test
required: true

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

c4d_version is a required: true workflow_call input, but release_publish.yml still calls both reusable workflows without a with: block (Xa11yWindows at .github/workflows/release_publish.yml:40 and Xa11yMacOS at :56). GitHub Actions fails the caller workflow at validation time when a required input is not supplied, so the release pipeline will break on its next run — PreRelease depends on both jobs.

Either add with: {c4d_version: "2026"} to those two release jobs (or a matrix, if release should validate all three), or give the input a default: "2026" and drop required: true.

push:
branches:
- mainline
- feature/ci-tests

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Adding feature/ci-tests as a push trigger here, plus the matching github.ref allowances in integ_windows.yml:29 and integ_macos.yml:29, permanently widens the set of refs that can run this privileged pipeline: these jobs assume AWS_OIDC_ROLE_ARN and LICENSE_ROLE_ARN and open an SSM tunnel to the shared license bastion. Unlike mainline, a feature/* branch is unlikely to carry branch protection or required reviews, so anyone with push access can trigger a credentialed run with arbitrary workflow content on that branch.

If this is scaffolding for generating the new render baselines, consider reverting these three hunks before merge and using workflow_dispatch (optionally with a c4d_version input) instead — that keeps the capability without hardcoding a long-lived unprotected branch into mainline. The docs added in test/AGENTS.md and skills/c4d-dev/references/integration-testing.md instruct pushing to feature/ci-tests, so they would need the same update.

Comment thread test/integ/conftest.py
f"Supported versions: {supported_versions}."
)

if "C4D_LOCATION" in os.environ:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

C4D_LOCATION now overrides the executable path but has no effect on the baseline directory, which is derived purely from C4D_VERSION (defaulting to 2026). So the previously documented workflow — setting only C4D_LOCATION to a non-default install, e.g. C:\Program Files\Maxon Cinema 4D 2025\ as the old docs showed — will launch Cinema 4D 2025 and then compare its renders against expected/renders/2026/. That fails as a golden mismatch with no hint that the version/baseline pair is inconsistent.

Worth making the mismatch detectable rather than relying on the docs, e.g. print the resolved (version, location) pair together, or infer/validate the version from the C4D_LOCATION directory name when it is recognizable and error out on disagreement.

"physical_tiles_multi_takes",
"phy_apos_path",
"redshift",
pytest.param("redshift", marks=_XFAIL_REDSHIFT_2024_2025_IN_CI),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

With strict=False, this marker makes both pass and fail acceptable, so on Cinema 4D 2024 and 2025 the redshift and redshift_tiles render comparisons are now entirely unenforced in CI — and the newly added baselines under redshift_tiles/expected/renders/2024|2025|2026/ are byte-identical to each other (all five PNGs share the same blobs across all three version directories). The same is true for redshift_textured, which is xfail on every version CI runs. Net effect: the new 2024/2025 Redshift goldens are committed but no CI configuration ever validates them, so a real regression or a wrong baseline is indistinguishable from the known Maxon issue.

If the 2024/2025 failures are genuinely intermittent rather than deterministic, a retry or a tolerance bump would preserve the signal; otherwise consider not committing baselines that nothing checks, so it is clear which version/case pairs are actually covered.

expected_dir / "renders",
resolve_expected_render_directory(
expected_dir,
os.environ.get("C4D_VERSION"),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The selected version is now read from two places with two different notions of a default. conftest.py:49 applies the 2026 default via os.environ.setdefault("C4D_VERSION", ...) at fixture time, but the xfail markers at lines 637/644 read os.environ.get("C4D_VERSION") at module import (collection) time, before any fixture runs, and this call site re-reads the raw env var rather than the resolved value.

Consequences: a local Windows run with C4D_VERSION unset compares against the 2026 baselines but sees None in the marker conditions, and if _DEFAULT_C4D_VERSION is ever bumped to 2027 the version sets in the markers silently stop matching the default. Since resolve_expected_render_directory raises on a falsy version, this also only works because the cinema4d_location fixture happens to have mutated os.environ first — an ordering dependency that is not obvious here.

Cleaner to resolve the version once (e.g. a cinema4d_version fixture, with the module-level markers keying off the same constant) and thread it through instead of round-tripping through the process environment.

for python_path in sys.path:
pywin32_dll_directory = os.path.join(python_path, "pywin32_system32")
if os.path.isdir(pywin32_dll_directory):
_dll_directory_handles.append(os.add_dll_directory(pywin32_dll_directory))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

sys.path can legitimately contain relative entries (notably "" for the current directory, and Cinema 4D injects its own entries). os.path.join("", "pywin32_system32") yields the relative path pywin32_system32, and os.path.isdir / os.add_dll_directory both resolve that against the process CWD — which during a render is a job working directory populated from job attachments. If an attacker-supplied bundle contains a pywin32_system32/ directory with a pywintypes311.dll, it lands on the DLL search path and gets loaded into the C4D process.

This only triggers when CINEMA4D_ADAPTOR_TESTING=true, so the exposure is limited to test runs, but it is cheap to close: skip non-absolute sys.path entries, e.g.

for python_path in sys.path:
    if not os.path.isabs(python_path):
        continue

Comment thread test/integ/test_utils.py
@@ -0,0 +1,22 @@
# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

These two tests need no Cinema 4D, no display, and no AWS — but living under test/integ/ means they only run via hatch run integ:test (hatch.toml), which is gated to mainline/feature/ci-tests pushes on self-hosted-ish runners with C4D installed. The PR-time Code Quality workflow runs test/unit only, so this coverage never executes on a pull request.

Moving the file to test/unit/ (importing test.integ.utils, which is import-safe — it only pulls in stdlib plus deadline_test_fixtures/yaml) would get it running on every PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting-on-maintainers Waiting on the maintainers to review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant