Skip to content
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

build: NGO CI update and rewrite [2.X] #3193

Merged
merged 69 commits into from
Feb 11, 2025

Conversation

michalChrobot
Copy link
Collaborator

@michalChrobot michalChrobot commented Jan 6, 2025

This PR contains a rewrite of CI present for NGO package (develop-2.0.0 branch. After merging this one a proper backport will be created for develop branch).

Below there is an overview of all changes, worth noting is that there are quite a few comments added in each file explaining what it does exactly, and why, so the PR description will be more about higher level information.

Beside yamato tests rewrite I needed to disable several tests in NetworkVariableTests for iOS/Android, I will investigate them after (worth noting is the fact that currently we are not tracking mobile tests so temporary disabling those tests on mobiles will not impact negatively our current CI).
Additionally I created pvpExceptions file in which we can track failing pvp tests (there is already a PR fixing PVP-150-1 failures). This will allow us to run pvp checks in our test suite without getting an exception about unreleased section present in changelog (this will fail only when trying to release but in this case we always update it to proper version)

  • .yamato/multiprocess-project-tests.yml

After consulting with @NoelStephensUnity the conclusion was that NGO is not using Multiprocess tests so those were removed.

  • .yamato/project-promotion.yml and .yamato/project-publish.yml

Those were removed since the release process changed and currently we use jobs present in wrench folder for package publishing and validation (described at the bottom of this summary together with other wrench jobs to showcase additional tests that are part of this CI)

  • .yamato/project-updated-dependencies-test.yml

This definition contained 2 trigger jobs.

First one was performing updated-dependencies-tests (which means that it was updating all package dependencies to the latest ones and running package tests). Currently we also plan to integrate renovate bot into this repository which will automatically detect dependencies that can be updated, create a PR with the update and package tests will run on it. In essence the difference is that Renovate will perform dependency updates and tests "one at a time" instead of all at the same time (worth noting that the test itself won't update the dependency but the bot will) but it should allow us to constantly use the latest dependencies.
We will keep this job though to be sure that we are compatible with updated packages (if any).

The second was a job using izon which is now unsupported (same think with Badges trigger in .yamato/triggers) so all badges tests were removed (their purpose was to add badges to GFithub repo)

  • .yamato/project.metafile

This file was rewritten in bigger part, the points to note are the following:

  1. small_agent_platform was created for definition of the test machine for jobs that for example doesn't require editor and are small (like pack job)
  2. test_platforms (which were desktop platforms) were expanded by adding sections like: default (platform to use when no specific platform is needed), desktop (desktop platforms), mobile_build and mobile_test (mobile definitions), console_build and console_test (console definitions)
  3. projects was shortened and some unused fields were removed
  4. Instead of having testeditors on per project basis there is now default validation editor (trunk) and other relevant test editors (for NGO v2.0.0 it's 6000.0+)
  5. win_il2cpp_test_image was removed since it wasn't relevant anymore
  • .yamato/package-pack.yml

Separate package-pack definition file was added. It's equivalent to wrench/package-pack-jobs and it packs the package and performs PVP checks on it. The idea here about having 2 package-pack definitions is that one is used by wrench (for release purposes) and other we can modify for our internal use.

  • .yamato/project-pack.yml

This definition was unified with the other in a sense that all machine definitions are present in project.metafile. Additional job of packing minimalproject with dependency of [email protected] (from old project.metafile) was removed since it would be out of date and in my understanding (as per previously mentioned Renovate bot) we should be able to keep latest dependency in projects (but I'm open to discussion if I'm wrong and this shouldn't be removed)

  • .yamato/package-tests.yml

This was minimally modified, for example the test will run only on default project (since we are testing package so there should be no difference of what project is used as a context). This is performed on all desktop platforms.

  • .yamato/project-tests.yml

Minimally rewritten similarly to package-tests. Ii will execute project tests for all projects that are marked with has_tests=true

  • .yamato/project-standards.yml

Modified in a way that it will run standards check on all projects (instead only on the first one). Here we mostly care about standards in the package which will be checked in any of projects but it's also nice to know if other projects are conforming to the standards.
Worth noticing is the fact that currently minimalproject and testproject-tools-integration will fail that test so it should be addressed after merging this PR. Only default project will be part of trigger test (nightly/weekly/PR) so this failure won't affect our workflow (it's just there to be aware of this).

  • .yamato/code-coverage.yml

Not much changed here. This job runs package test with code coverage measurement enabled. It's automatically triggered once per week by trigger jobs definition. The results are not shared internally since it's not supported for public repositories (yet).

  • .yamato/webgl-build.yml

Also here the functionality stayed mostly the same. It will perform build phase for WebGL and no tests since it's not really feasible right now to run server for such build.

  • .yamato/standalone-project-tests.yml was replaced by .yamato/desktop-standalone-tests.yml

The definition (beside some different writing) stayed mostly the same and those jobs are created for each supported editor for each desktop platform. Job was renamed for better readability.

  • .yamato/mobile-build-and-test.yml was replaced by .yamato/mobile-standalone-test.yml

The definition (beside some different writing) stayed mostly the same and those jobs are created for each supported editor for each desktop platform. Job was renamed for better readability.

  • .yamato/console-standalone-test.yml

This is a completely new job since there were no console tests set up before for NGO package

  • .yamato/performance-tests.yml

This is a completely new job since there were no performance tests set up before for NGO package. Worth noting is the fact that currently NGO doesn't have any performance tests in place (so this job will always fail with "no tests selected" error) but it's still nice to have this job for future when there will be performance tests. Because of this, this job will not be triggered by any trigger jobs.

  • .yamato/_run-all.yml

It was modified in a sense that it runs jobs in current configuration. Worth noting is that there are jobs to run all given tests (for example mobile) or all but while only using default editor (trunk). This is useful for triggers defined in .yamato/_triggers.yml

  • .yamato/_triggers.yml

First of all "Badges Tests Trigger" was removed because izon is not supported anymore as mentioned also in .yamato/project-updated-dependencies-test.yml.

Pull Request Trigger was modified to run only on valid branches (there is no master branch) . For now it will run project-standards, preview-a-p-v, api-validation-jobs and all package and project tests.

Run Multiprocess Tests trigger was removed since there are no multiprocess jobs anymore

[Nightly] Run All Tests was modified to run same subset as pull_request_trigger with addition of mobile/desktop/console tests and webgl builds while running on trunk (default editor)

[Weekly] Run All Tests was modified to run same subset as develop_nightly but runs per all supported editors as well as executes code coverage test and runs project standards for default project (so failure of other projects mentioned before won't affect it. We are either way focusing on package so this test will be enough)

  • Summary of Wrench jobs

Those jobs were created before this PR but I will summarize those just to have a full overview of jobs possible to use. All of those are present under .yamato/wrench folder:

  • api-validation-jobs --> This job validates package API to make sure that there are no breaking changes when major package version is not bumped.
  • package-pack-jobs --> Equivalent to .yamato/package-pack.yml but due to wrench dependencies it was kept and .yamato/package-pack.yml is used for other test jobs.
  • preview-a-p-v --> This job validates if our package changes are not breaking dependents of our package
  • promotion-jobs --> This is equivalent to removed promotion and publish jobs
  • validation-jobs --> Jobs validating PVP checks as well as EditMode and PlayMode tests on different platforms

@michalChrobot michalChrobot self-assigned this Jan 6, 2025
@michalChrobot michalChrobot mentioned this pull request Jan 8, 2025
@michalChrobot michalChrobot changed the title NGO CI update and rewrite ci: NGO CI update and rewrite Jan 8, 2025
michalChrobot and others added 7 commits February 6, 2025 11:04
…elop-2.0.0 earlier yesterday (this makes sure that configuration will be the same between branches)
Seeing if using the v5 vs the hash fixes an issue with stalls and/or bypassing the trigger process.
This reverts commit 302ba7c.
Seeing if this triggers a full build
@NoelStephensUnity NoelStephensUnity changed the title ci: NGO CI update and rewrite [2.X] chore: NGO CI update and rewrite [2.X] Feb 6, 2025
@michalChrobot michalChrobot added the port:1.x-needed This issue needs to be ported to 1.X branch label Feb 7, 2025
@NoelStephensUnity NoelStephensUnity changed the title chore: NGO CI update and rewrite [2.X] build: NGO CI update and rewrite [2.X] Feb 7, 2025
@michalChrobot michalChrobot merged commit be45e86 into develop-2.0.0 Feb 11, 2025
27 checks passed
@michalChrobot michalChrobot deleted the NGO-CI-rewrite-for-develop-2.0.0 branch February 11, 2025 09:36
michalChrobot added a commit that referenced this pull request Feb 11, 2025
* Refactor of CI setup, jobs and triggers
* Disabling some tests which failing on mobile devices
* Setting up Renovate bot configuration (same as already present in develop branch)
* Added pvpExceptions file

---------

Co-authored-by: NoelStephensUnity <[email protected]>
NoelStephensUnity added a commit that referenced this pull request Feb 13, 2025
* build: NGO CI update and rewrite [2.X] (#3193)

* Refactor of CI setup, jobs and triggers
* Disabling some tests which failing on mobile devices
* Setting up Renovate bot configuration (same as already present in develop branch)
* Added pvpExceptions file

---------

Co-authored-by: NoelStephensUnity <[email protected]>

* Updated pvpExceptions file and moved it to proper location

* Added pvpExceptions file into package folder

* Updated editor coverage

* Updated pvpExceptions meta file

* Corrected pvpExceptions meta

* Fixed Standard check error saying "'NetworkDriver.RemoteEndPoint(NetworkConnection)' is obsolete: 'RemoteEndPoint has been renamed to GetRemoteEndpoint. (UnityUpgradable) -> GetRemoteEndpoint(*)'"

* Revert "Fixed Standard check error saying "'NetworkDriver.RemoteEndPoint(NetworkConnection)' is obsolete: 'RemoteEndPoint has been renamed to GetRemoteEndpoint. (UnityUpgradable) -> GetRemoteEndpoint(*)'""

This reverts commit 7ccdd89.

* fix

NGO v1.x compatibility for UTP v2.x

* fix

Minor adjustment for the legacy NetworkEndPoint name.

* fix

More UTP v2x related (collections etc.) defines required.

* fix

Missed one of the files

* Corrected trigger branch for nightly and weekly jobs

* Added testing coverage on minimal supported editor (2021.3)

* Corrected typo in runAll configurtation

* Corrected platform name in webgl definition

* Added missing tests coverage for project-updated-dependencies

* Corrected coverage

* update

Updating some integration tests and some additional areas that were problematic to IL2CPP integration tests.

* test  - fix

This resolves the trunk failures due to updates in collections.

* Changed PR trigger to use mono for 2021 build

* Added libssl install step

* Updated libssl version

* Changed build for 2021.3 editor to win

---------

Co-authored-by: NoelStephensUnity <[email protected]>
@michalChrobot michalChrobot added port:1.x-completed This issue was ported to 1.X branch and removed port:1.x-needed This issue needs to be ported to 1.X branch labels Feb 14, 2025
EmandM added a commit that referenced this pull request Mar 24, 2025
This PR focuses on fixing PVP errors related to PVP-151-1 "**_Public
APIs should be documented_**" and all remaining PVP errors **present in
pvpExceptions.json file**

We have quite a lot of errors to address so this PR will be a
collaborative effort. If anyone want's to add to it what we should do
is:

1. Check errors present in pvpExceptions.json file (preferably under
"PVP-150-1", choose some to fix
2. Address those errors and when making commit remove the fixed errors
from pvpExceptions file
The first commit of this PR is an example of how such process should
look like

In general [new CI
implementation](#3193)
will catch all new errors like this so all we need to do is fix the
present ones. This PR **does not** focus on PVP present in gold profile
but if capacity allows, those can also be addressed.

**Error present in "PVP-41-1" is to be expected** so there is no need of
fixing this one!

Another thing is that we could ignore error related to missing APIs for
RuntimeTests because those were made internal in 2.X version (that's why
those errors are not popping by there). Those APIs are not user facing
so we could just leave them (otherwise there is like 2000 more APIs to
add)

---------

Co-authored-by: Noel Stephens <[email protected]>
Co-authored-by: Emma <[email protected]>
Co-authored-by: unity-renovate[bot] <120015202+unity-renovate[bot]@users.noreply.github.com>
EmandM added a commit that referenced this pull request Mar 24, 2025
This PR focuses on fixing PVP errors related to PVP-151-1 "**_Public
APIs should be documented_**" and all remaining PVP errors **present in
pvpExceptions.json file**

We currently have 768 errors to address so this PR will be a
collaborative effort. If anyone want's to add to it what we should do
is:

1. Check errors present in pvpExceptions.json file (preferably under
"PVP-150-1", choose some to fix
2. Address those errors and when making commit remove the fixed errors
from pvpExceptions file

In general [new CI
implementation](#3193)
will catch all new errors like this so all we need to do is fix the
present ones. This PR **does not** focus on PVP present in gold profile
but if capacity allows, those can also be addressed.

**Error present in "PVP-41-1" is to be expected** so there is no need of
fixing this one!

---------

Co-authored-by: Noel Stephens <[email protected]>
Co-authored-by: Emma <[email protected]>
NoelStephensUnity pushed a commit that referenced this pull request Apr 11, 2025
This PR is a continuation of
#3225
and focuses on fixing PVP errors related to PVP-151-1 "**_Public APIs
should be documented_**" and all remaining PVP errors **present in
pvpExceptions.json file**

We currently have 137 errors to address so this PR will be a
collaborative effort. If anyone want's to add to it what we should do
is:

1. Check errors present in pvpExceptions.json file (preferably under
"PVP-150-1", choose some to fix
2. Address those errors and when making commit remove the fixed errors
from pvpExceptions file

In general [new CI
implementation](#3193)
will catch all new errors like this so all we need to do is fix the
present ones. This PR **does not** focus on PVP present in gold profile
but if capacity allows, those can also be addressed.

**Error present in "PVP-41-1" is to be expected** so there is no need of
fixing this one!
Another thing is that we could ignore error related to missing APIs for
tests (so TestHelpers, EditorTests, RuntimeTests) because those are not
affecting users in any way.

## Backport 
This PR has its equivalent in the sense of
#3374
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
port:1.x-completed This issue was ported to 1.X branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants