Skip to content

Conversation

@DellaBitta
Copy link
Contributor

@DellaBitta DellaBitta commented Jan 17, 2023

Description

Provide details of the change, and generalize the change in the PR title above.

TLDR; Instead of fixing the build matrix in our workflows, which is quite rigid across all targets, this change updates the build_testapps.py to suppress its build of Dynamic Links even if its passed in as one the provided targets on tvOS.

The Unity SDK build workflow supplies the Integration tests workflow with the full list of SDKs to test against as part of the nightly build process. This has traditionally been fine since all our SDKs were supported by all build targets.

However, tvOS doesn't support Dynamic Links. When fed the entirety of SDKs as build targets the build_testapps.py script would build the testapp a tvOS Dynamic Links testapp. The resulting artfiact would be a shell of an application with the C# definitions (as they're shared across tvOS and iOS in the .dlls) but with no C++ implementation. The test_simulator.py script would then find this errant testapp and blindly attempt to run it. However, the testapp would error-out upon execution due to the lack of C++ symbols.

This change suppresses the build of the Dynamic Links test app in build_testapps.py. With no testapp built, test_simualtor.py is ignorant of the fact that one test is missing, and runs all of the others successfully.


Testing

Describe how you've tested these changes.

Integration Test CI


Type of Change

Place an x the applicable box:

  • Bug fix. Add the issue # below if applicable.
  • New feature. A non-breaking change which adds functionality.
  • Other, such as a build process or documentation change.

@DellaBitta DellaBitta requested a review from a-maurice January 18, 2023 16:23
@DellaBitta DellaBitta marked this pull request as ready for review January 18, 2023 16:23
@DellaBitta DellaBitta merged commit 0d72662 into main Jan 18, 2023
@DellaBitta DellaBitta deleted the feature/tvos_build_testapps branch January 18, 2023 19:31
DellaBitta added a commit that referenced this pull request Jan 19, 2023
I introduced a bug in our build system with PR #599 where I added an explicit list of build targets for each SDK's testapp. In this change I errantly listed "Desktop" as a possible target, but really desktop is either "Windows, "Linux", or "macOS".

This PR fixes the issue by expanding Desktop entries to those values.
@firebase firebase locked and limited conversation to collaborators Feb 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants