feat(ci): add automated testing and release workflows#553
Closed
JasonXuDeveloper wants to merge 23 commits intomasterfrom
Closed
feat(ci): add automated testing and release workflows#553JasonXuDeveloper wants to merge 23 commits intomasterfrom
JasonXuDeveloper wants to merge 23 commits intomasterfrom
Conversation
This commit fixes an issue where play mode tests would jump to the init scene but not run any tests. Changes: - Wrap TestRunnerCallbacks.IsRunningTests check with #if UNITY_INCLUDE_TESTS in ChangeScene.cs This ensures the test runner check is only performed when test framework is available - Add test assembly reference to JEngine.Core.Editor.asmdef (GUID:0acc523941302664db1f4e527237feb3) - Update .gitignore to exclude Claude-generated files The root cause was that ChangeScene.cs was trying to check TestRunnerCallbacks.IsRunningTests without verifying the test framework was available, which could cause the check to fail or not work properly during play mode tests. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…dMethod registration The previous fix checked TestRunnerCallbacks.IsRunningTests at runtime, but there was a timing issue - RuntimeInitializeOnLoadMethod executes before TestRunnerCallbacks.RunStarted is called, so the flag was not set yet. This commit uses the cleaner approach recommended by Unity: prevent the method from being registered at all during tests by applying #if !UNITY_INCLUDE_TESTS to the attribute itself. This eliminates the runtime check and TestRunnerCallbacks dependency entirely from ChangeScene.cs, making the code simpler and more reliable. References: - Unity Discussions: RuntimeInitializeOnLoadMethod runs during playmode unit tests https://discussions.unity.com/t/methods-with-runtimeinitializeonloadmethod-attribute-run-during-playmode-unit-tests/949876 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…dent callbacks The previous approaches had timing issues - TestRunnerCallbacks.RunStarted is called after RuntimeInitializeOnLoadMethod, and #if !UNITY_INCLUDE_TESTS doesn't work because the assembly always has UNITY_INCLUDE_TESTS defined (it contains test files). This commit uses a simple, reliable approach: Unity Test Framework creates temporary scenes with names like "InitTestScene<timestamp>" when running play mode tests. By checking if the current scene name starts with "InitTestScene", we can reliably detect test runs at runtime without timing dependencies. Benefits: - No timing issues with callbacks - No dependency on preprocessor directives - Works in all scenarios (GUI test runner, command line, etc.) - Simple and maintainable Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Unity Test Framework creates temporary scenes like InitTestScene<timestamp>.unity when running play mode tests. These should not be committed to the repository. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…ode clarity - Simplified lambda expressions by removing explicit parameter types - Added missing using statements for System.Collections.Generic and System.Reflection - Removed redundant System namespace prefixes - Changed DelayFrame(1) to DelayFrame() for default behavior - Minor code cleanup for better readability Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Fixes two critical issues in the Panel build process: 1. Build Error Detection: - Add error tracking to detect when code build fails - Track Unity error count before and during build - Stop asset build if code build fails - Throw exceptions when menu items fail to execute - Add try-catch blocks around compile/obfuscate operations 2. Progress Bar Updates: - Implement UpdateProgressAsync() to force UI repaints - Use EditorApplication.delayCall for async updates - Progress bar now updates during blocking operations Changes: - Add _buildErrorOccurred and _errorCountAtBuildStart fields - Add GetUnityErrorCount() to query Unity's LogEntries - Add CheckForBuildErrors() to validate build steps - Add UpdateProgressAsync() for responsive progress updates - Update BuildAll() to abort if code build fails - Update BuildCodeOnly() with same error handling - Update BuildCode() to check errors after each step - Update ExecuteMenuItem() to throw on failure Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Refactors the build process to be non-blocking and provide real-time progress updates using Unity's recommended patterns. Implementation: 1. **State Machine Pattern** (EditorApplication.update): - Enum-based build steps (GenerateEncryptionVM → Complete) - Each step executes in separate editor frame - UI remains responsive throughout build - Proper cleanup on completion/failure 2. **Progress API Integration** (Unity 2022.1+): - Progress.Start() creates tracked operation - Progress.Report() updates with description - Progress.Finish() with success/failure status - Shows in Unity's Progress window - Supports cancellation 3. **Triple Progress Visualization**: - EditorUtility.DisplayProgressBar (modal progress) - Progress API (background tracking) - UI Toolkit ProgressBar (panel integration) 4. **Build Step Execution**: - GenerateEncryptionVM (10%) - GenerateSecretKey (20%) - GenerateAll (30%) - CompileDll (40%) - ObfuscateCode (45%) - CopyAssemblies (50%) - BuildAssets (60-100%, if BuildAll) - Complete (cleanup) Key Benefits: - Build process no longer blocks Unity editor - Progress bar updates in real-time during all operations - Build can be cancelled via Progress window - Errors detected and reported immediately - BuildAll stops if code build fails - Clean state machine pattern for maintainability Research-based on Unity patterns found in: - NavMeshAssetManager (Progress API + AsyncOperation) - ShaderVariantCollector (State machine pattern) - YooAsset build pipeline (Multi-phase operations) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Major code quality improvement by separating concerns and reducing Panel.cs file size by 42% (from ~880 lines to 511 lines). Changes: 1. **New BuildManager Class** (473 lines): - Encapsulates all build logic in single-responsibility class - Manages build state machine - Handles code compilation, obfuscation, asset bundling - Provides clean callback-based API for UI integration - Contains all helper methods (CopyAssemblies, CreateBuildParameters, etc.) 2. **Refactored Panel.cs** (511 lines): - Now focused solely on UI presentation - Simplified BuildAll/BuildCodeOnly to delegate to BuildManager - Removed duplicate code and build state management - Cleaner callback-based completion/error handling 3. **Fixed Compiler Warnings**: - Removed unused _buildErrorOccurred field (CS0414) - Properly using exception-based error handling instead Benefits: - **Single Responsibility**: Panel = UI, BuildManager = Build Logic - **Maintainability**: Easier to test and modify build process - **Readability**: Each file has clear, focused purpose - **Reusability**: BuildManager can be used from other contexts - **Less Coupling**: UI and build logic properly separated File Size Comparison: - Before: Panel.cs ~880 lines (everything in one file) - After: Panel.cs 511 lines + BuildManager.cs 473 lines - Reduction: 42% smaller Panel.cs, better organized code Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Add StartBuildAssetsOnly method to BuildManager for assets-only builds - Update Panel.BuildAssetsOnly to use BuildManager instead of local state - Resolves CS0103 errors about missing _isBuilding, GetNextPackageVersion, and BuildAssets Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
The in-panel ProgressBar was redundant since we already have: - EditorUtility.DisplayProgressBar (modal dialog) - Unity Progress API (unified progress tracking) - Log messages showing current step Changes: - Remove _progressBar field and UI element from Panel.cs - Remove UpdateProgress callback from BuildManager constructor - Simplify build methods (BuildAll, BuildCodeOnly, BuildAssetsOnly) - Progress is still tracked via EditorUtility.DisplayProgressBar and Progress API This reduces code complexity and eliminates the non-working progress bar the user reported. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…e from obfuscation JEngine.Core is framework code that uses reflection to dynamically load hot update assemblies. It should not be obfuscated since: 1. It's part of the AOT runtime, not hot update code 2. It uses Type.GetMethod() reflection which triggers compatibility warnings 3. Framework code doesn't benefit from obfuscation Changes: - Move JEngine.Core from 'assembliesToObfuscate' to 'nonObfuscatedButReferencingObfuscatedAssemblies' - This preserves the reflection code while still allowing it to reference obfuscated assemblies like HotUpdate.Code This eliminates the ReflectionCompatibilityDetector warnings about Bootstrap.LoadHotCode() without needing attribute workarounds.
…gine.Core from obfuscation" This reverts commit 4aaddaf.
YooAsset's PreprocessBuildCatalog throws a NullReferenceException: 'Create package main catalog file failed ! Object reference not set to an instance of an object' This error occurs during the HybridCLR/ObfuzExtension/GenerateAll step but doesn't actually break the build - it's a bug in YooAsset's catalog generation. Changes: - Comment out CheckForErrors() call for BuildStep.GenerateAll - Add TODO comment explaining the temporary workaround - Build can now proceed past this non-blocking error The error is logged to console but won't stop the build process.
Instead of skipping error checking entirely, implement CheckForErrorsExcluding() which reads Unity console log entries and filters out specific known errors. The YooAsset catalog error 'Create package main catalog file failed' is a known non-blocking bug that doesn't actually break the build. This change: 1. Adds CheckForErrorsExcluding(context, excludeMessageContaining) method 2. Uses reflection to read Unity LogEntries and check error messages 3. Counts only non-excluded errors 4. Logs a note when known errors are ignored but continues the build 5. Throws exception only if real (non-excluded) errors occur Changes in ExecuteCurrentStep: - BuildStep.GenerateAll now uses CheckForErrorsExcluding() instead of CheckForErrors() - Specifically excludes 'Create package main catalog file failed' errors - All other errors will still stop the build This provides proper error handling while working around the YooAsset bug.
Added HybridCLR/ObfuzExtension/GeneratePolymorphicCodes menu item execution after the GenerateAll step. Changes: - Add BuildStep.GeneratePolymorphicCodes enum value - Add case in ExecuteCurrentStep for Step 4/5: Generating Polymorphic Codes - Update step numbering from X/4 to X/5 throughout build process: - GenerateAll: 3/5 - GeneratePolymorphicCodes: 4/5 (new) - CompileDll: 5/5 - ObfuscateCode: 5/5 - CopyAssemblies: 5/5 - Add progress update at 0.35 for the new step - Execute HybridCLR/ObfuzExtension/GeneratePolymorphicCodes menu item - Apply standard error checking (CheckForErrors) for this step Build flow now: 1. GenerateEncryptionVM 2. GenerateSecretKey 3. GenerateAll 4. GeneratePolymorphicCodes (new) 5. CompileDll 6. ObfuscateCode 7. CopyAssemblies 8. BuildAssets (if buildAll)
GenerateAll menu item (HybridCLR/ObfuzExtension/GenerateAll) internally calls: 1. CompileDllCommand.CompileDll(target) 2. GeneratePolymorphicCodesWhenEnable() 3. ObfuscateUtil.ObfuscateHotUpdateAssemblies(target, obfuscatedHotUpdateDllPath) So we were duplicating these operations: - BuildStep.GeneratePolymorphicCodes ❌ (redundant) - BuildStep.CompileDll ❌ (redundant) - BuildStep.ObfuscateCode ❌ (redundant) Simplified build flow: 1. GenerateEncryptionVM (Step 1/3) 2. GenerateSecretKey (Step 2/3) 3. GenerateAll (Step 3/3) - does compile, polymorphic gen, obfuscation 4. CopyAssemblies 5. BuildAssets (if buildAll) Changes: - Remove BuildStep enum values: GeneratePolymorphicCodes, CompileDll, ObfuscateCode - Remove corresponding cases in ExecuteCurrentStep() - Update step numbering from 5 steps to 3 steps - Add explanatory log message for GenerateAll step - Keep CopyAssemblies which copies obfuscated DLLs to Assets/HotUpdate/Compiled This reduces build complexity and eliminates duplicate compilation/obfuscation.
While GenerateAll calls GeneratePolymorphicCodesWhenEnable() internally, it only runs conditionally based on settings. Add explicit call to ensure polymorphic codes are always generated. Changes: - Add BuildStep.GeneratePolymorphicCodes back to enum - Add Step 4/4: Generating Polymorphic Codes case - Execute HybridCLR/ObfuzExtension/GeneratePolymorphicCodes menu item - Update step numbering from 3 to 4 steps - Update progress from 0.3 (GenerateAll) to 0.4 (GeneratePolymorphicCodes) Build flow: 1. GenerateEncryptionVM (Step 1/4) 2. GenerateSecretKey (Step 2/4) 3. GenerateAll (Step 3/4) - compiles and obfuscates 4. GeneratePolymorphicCodes (Step 4/4) - ensures polymorphic code generation 5. CopyAssemblies 6. BuildAssets (if buildAll)
After extracting BuildManager, these imports are no longer needed: - Panel.cs: Remove unused HybridCLR, Obfuz, YooAsset editor imports - EditorUIUtils.cs: Remove unused UnityEditor.UIElements import - Simplify lambda expressions (e) => to e => No functional changes, only code cleanup.
Generated files from running the updated build system with: - New BuildManager state machine - Updated build steps (4 steps instead of 5) - Selective error filtering for YooAsset catalog bug - Explicit GeneratePolymorphicCodes execution All compiled DLLs, AOT assemblies, HybridCLR generated code, obfuscation mappings, and asset bundles have been regenerated.
CodeFactor identified 10 maintainability issues: 1. Arithmetic operator precedence (BuildManager.cs:432) - Added explicit parentheses for clarity in version calculation 2. Multiple consecutive blank lines (3 issues) - Panel.cs:273 - removed extra blank line - EditorUIUtils.cs:284 - removed extra blank line - EditorUIUtils.cs:332 - removed extra blank line 3. Documentation header formatting (Panel.cs:433) - Removed blank line between XML comment and method All issues were minor severity maintainability concerns.
1. Add XML documentation for IsBuilding property - Document that it indicates whether a build is in progress 2. Fix StartBuildAssetsOnly progress initialization - Add Progress.Start() to initialize _progressId - Add Progress.Finish() in try/catch blocks - Add EditorUtility.ClearProgressBar() in finally block - Matches pattern used in StartBuildAll and StartBuildCodeOnly
Implement comprehensive CI/CD automation for JEngine: - Add reusable Unity test workflow with EditMode/PlayMode support - Add PR testing workflow with automatic test result comments - Add release automation workflow with dual-package version bumping - Add automatic changelog generation from conventional commits - Add GitHub App authentication for secure bot commits - Add conventional commit guidelines to CLAUDE.md - Add PR template to guide contributors on commit format This enables: - Automated Unity testing on every PR - One-click releases with automatic version updates - Automatic changelog generation and GitHub releases - OpenUPM integration for package distribution Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Unity Test Results✅ EditMode: All tests passed Unity Version: 2022.3.55f1 ✅ All tests passed! The PR is ready for review. View workflow run |
Code reviewI found one issue that should be addressed: Missing XML DocumentationFile: The public constructor is missing required XML documentation. According to CLAUDE.md, all public APIs must be documented with XML comments including Suggested fix: /// <summary>
/// Initializes a new instance of the BuildManager class.
/// </summary>
/// <param name="settings">The build settings to use.</param>
/// <param name="logCallback">Callback for logging build messages.</param>
public BuildManager(Settings settings, Action<string, bool> logCallback)
{
_settings = settings;
_logCallback = logCallback;
} |
Owner
Author
|
Closing this PR - branch was based from wrong branch. Will reopen with correct base from master. |
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR implements comprehensive CI/CD automation for JEngine, enabling automated testing on PRs and one-click releases with automatic changelog generation.
Type of Change
Package Scope
com.jasonxudeveloper.jengine.core)com.jasonxudeveloper.jengine.util)What's Included
1. Automated PR Testing
2. Release Automation
3. Developer Guidelines
Benefits
✅ Automated Testing: Every PR runs Unity tests automatically
✅ Quality Control: Tests must pass before merge
✅ Easy Releases: One-click releases with automatic version bumping
✅ Auto Changelog: Generated from conventional commits
✅ OpenUPM Integration: Automatic package publishing
✅ Consistent Commits: Guidelines for contributors
Testing
This PR itself will test the PR workflow:
After merge, the release workflow can be tested manually via Actions tab.
Test environment:
Additional Notes
Prerequisites (already completed):
First run timing:
Post-merge tasks:
🤖 This PR was prepared with assistance from Claude Code