-
Notifications
You must be signed in to change notification settings - Fork 261
Add RunPageScriptingTests parameter and implementation to Run-AlPipeline #4052
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
base: main
Are you sure you want to change the base?
Conversation
…ine microsoft#2001 * Pass the parameters as a hashtable to RunPageScriptingTests function so that they can be adjusted later in a non-breaking manner. * do a depper check on bcAuthContext to differ saas envionments from other. * do not check if the container was lef created before running page scripting tests.
freddykristiansen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 questions / comments
| Write-GroupEnd | ||
| } | ||
|
|
||
| if ($createContainer -and !$doNotRunPageScriptingTests -and $pageScriptingTests -and $pageScriptingTestResultsFolder -and $pageScriptingTestResultsFile) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would you not check that a container is created when restoring the database in a container?
Else, I assume this could fail with CompilerFolder running
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just because you use CompilerFolder doesn't necessarily mean you don't have a container.
That's only the case if you set doNotPublishApps at the same time. But if you set this, doNotRunPageScriptingTests is automatically set to true. That would make it OK again, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR enhances the Run-AlPipeline function by adding the ability to override the page scripting tests execution logic and improves SaaS environment detection. The key changes include:
- Refactoring page scripting test code into a reusable function that accepts parameters as a hashtable for future extensibility
- Improving SaaS infrastructure detection with a more thorough check on
bcAuthContext - Removing the
$createContainercheck for running page scripting tests, allowing them to run against existing containers
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| ReleaseNotes.txt | Added release note for version 6.1.10 documenting the SaaS environment recognition improvement and page scripting test override capability |
| AppHandling/Run-AlPipeline.ps1 | Added RunPageScriptingTests parameter and function, improved SaaS detection logic, refactored page scripting tests execution, and removed container creation requirement |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
…nctionality; update ReleaseNotes for environment recognition enhancement
Improved SaaS Environment recognition and allowed overwriting Page Scripting Test execution in Run-AlPipeline.
fixes AL-Go#2001