Fix legacy json import compatibility, UI sidebar refactor, and improved test coverage - #4911
Fix legacy json import compatibility, UI sidebar refactor, and improved test coverage#4911yadavnikhil03 wants to merge 4 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change moves app-category UI population into ChangesUI initialization and import handling
Variable discovery test coverage
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
pester/lazy-tabs.Tests.ps1 (1)
42-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRestore runtime coverage for app-category rendering.
The removed assertion no longer verifies that
Initialize-WPFUI -TargetGridName "appscategory"callsInvoke-WPFUIElementswith$sync.configs.appnavigationand-columncount 1. Add a focused test forInitialize-WPFUI; keep this test focused on one-time tab initialization.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pester/lazy-tabs.Tests.ps1` at line 42, Add a focused test for Initialize-WPFUI targeting "appscategory" that verifies Invoke-WPFUIElements receives $sync.configs.appnavigation with -columncount 1, and keep the test limited to one-time tab initialization behavior.functions/public/Invoke-WPFImpex.ps1 (1)
76-85: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression tests for both import shapes.
The provided tests do not execute this compatibility branch. Add Pester cases for a legacy object containing
WPFTweaks,WPFFeature, andInstall, and for a flat array. MockUpdate-WinUtilSelectionsand assert that only selection keys reach it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@functions/public/Invoke-WPFImpex.ps1` around lines 76 - 85, Add Pester regression cases covering the compatibility branch in Invoke-WPFImpex: one legacy PSCustomObject containing WPFTweaks, WPFFeature, and Install, and one flat string array. Mock Update-WinUtilSelections, invoke the import flow for each shape, and assert that only WPFTweaks and WPFFeature selection values are passed, excluding Install.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pester/variables.Tests.ps1`:
- Around line 5-22: Preserve the original process-wide value of $global:sync
before replacing it in BeforeAll, then restore that saved value during the
test-file cleanup, such as AfterAll or the existing Describe cleanup scope.
Update the setup and cleanup around the Get-WinUtilVariables test so later
Pester files observe the original $global:sync state.
---
Nitpick comments:
In `@functions/public/Invoke-WPFImpex.ps1`:
- Around line 76-85: Add Pester regression cases covering the compatibility
branch in Invoke-WPFImpex: one legacy PSCustomObject containing WPFTweaks,
WPFFeature, and Install, and one flat string array. Mock
Update-WinUtilSelections, invoke the import flow for each shape, and assert that
only WPFTweaks and WPFFeature selection values are passed, excluding Install.
In `@pester/lazy-tabs.Tests.ps1`:
- Line 42: Add a focused test for Initialize-WPFUI targeting "appscategory" that
verifies Invoke-WPFUIElements receives $sync.configs.appnavigation with
-columncount 1, and keep the test limited to one-time tab initialization
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e09fe5cc-2220-44c1-866b-e2d0117e4de1
📒 Files selected for processing (6)
functions/private/Initialize-WinUtilTabContent.ps1functions/public/Initialize-WPFUI.ps1functions/public/Invoke-WPFImpex.ps1pester/configs.Tests.ps1pester/lazy-tabs.Tests.ps1pester/variables.Tests.ps1
💤 Files with no reviewable changes (1)
- functions/private/Initialize-WinUtilTabContent.ps1
ea151ea to
7dab1ee
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
Thanks for looking into this. I agree that the legacy JSON format needs to be addressed, but I have some concerns with the current import change. The importer currently explicitly skips the old Could you add a small legacy JSON fixture, or otherwise document the exact config and expected result used for validation? Right now it's difficult for a reviewer to reproduce this manually, and the existing tests don't appear to validate the new legacy-import path. I don't want to over-focus on the unrelated refactors here, but before calling this issue resolved, I think the expected migration behavior needs to be agreed on and tested, especially around the old |
|
Oh, good point @mewclouds and one I feared, that in the current state by not IDing the package sources some applications will not be installed on the device - and there are no warnings or communications to let the end user know this is the state of things. |
3 participants
Type of Change
Description
fixes a couple of TODOs from the codebase and adds some missing test coverage.
Invoke-WPFImpex.ps1to detect and flatten oldPSCustomObjectconfigs so legacy backups import properly instead of silently failing.appscategoryUI generation out ofInitialize-WinUtilTabContent.ps1and intoInitialize-WPFUI.ps1where it actually belongs.lazy-tabsandconfigs) that broke from the UI move.Get-WinUtilVariablessince it had no coverage.compiled locally and all 475 pester tests are passing green.
Issue related to PR