test: Run integration tests across Cinema 4D versions (2024,2025,2026) - #513
test: Run integration tests across Cinema 4D versions (2024,2025,2026)#513karthikbekalp wants to merge 3 commits into
Conversation
Signed-off-by: Karthik Bekal Pattathana <133984042+karthikbekalp@users.noreply.github.com>
|
Resolving this in favor of : #515 |
Pull request was closed
| inputs: | ||
| c4d_version: | ||
| description: Cinema 4D major version to test | ||
| required: true |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
| f"Supported versions: {supported_versions}." | ||
| ) | ||
|
|
||
| if "C4D_LOCATION" in os.environ: |
There was a problem hiding this comment.
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), |
There was a problem hiding this comment.
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"), |
There was a problem hiding this comment.
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)) |
There was a problem hiding this comment.
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| @@ -0,0 +1,22 @@ | |||
| # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | |||
There was a problem hiding this comment.
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.
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.