Perl ts integration#1362
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/CommunityToolkit/Aspire/main/eng/scripts/dogfood-pr.sh | bash -s -- 1362Or
iex "& { $(irm https://raw.githubusercontent.com/CommunityToolkit/Aspire/main/eng/scripts/dogfood-pr.ps1) } 1362" |
There was a problem hiding this comment.
Pull request overview
This PR extends the existing CommunityToolkit.Aspire.Hosting.Perl integration to support Aspire’s TypeScript/polyglot authoring flow by exporting Perl builder APIs for TS generation, adding a TypeScript AppHost example + tests, and enhancing the shared TypeScript AppHost validation tooling to support more validation scenarios (configured package mappings, optional startup, and HTTP probing).
Changes:
- Add
AspireExportattributes to Perl builder extension methods so they can be consumed from generated TypeScript AppHosts. - Add a Perl TypeScript AppHost example (
cpanm-api-integration) plus new tests that validate startup and endpoint behavior via HTTP probes. - Enhance the TypeScript AppHost validation harness (C# + PowerShell) with
-UseConfiguredPackages,-SkipStart, optional AppHost path override, and post-start HTTP probing.
Reviewed changes
Copilot reviewed 16 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/CommunityToolkit.Aspire.Testing/TypeScriptAppHostTest.cs | Extends shared TS AppHost test runner with configured-packages, skip-start, HTTP probe, and custom path options. |
| tests/CommunityToolkit.Aspire.Hosting.Perl.Tests/WithCertificateTrustTests.cs | Adds a propagation test for Perl certificate trust when using project dependency installers. |
| tests/CommunityToolkit.Aspire.Hosting.Perl.Tests/TypeScriptAppHostTests.cs | New Perl TypeScript AppHost integration tests using HTTP probes. |
| tests/CommunityToolkit.Aspire.Hosting.Perl.Tests/AddPerlExecutableTests.cs | Adds coverage ensuring AddPerlExecutable doesn’t register interpreter-related required commands. |
| src/CommunityToolkit.Aspire.Hosting.Perl/PerlAppResourceBuilderExtensions.PackageManager.cs | Adds AspireExport for package manager-related fluent APIs. |
| src/CommunityToolkit.Aspire.Hosting.Perl/PerlAppResourceBuilderExtensions.cs | Adds AspireExport for AddPerl*/Perl fluent APIs; adjusts required-command registration for executable entrypoints. |
| src/CommunityToolkit.Aspire.Hosting.Perl/CommunityToolkit.Aspire.Hosting.Perl.csproj | Suppresses ASPIREATS001 for using experimental AspireExport. |
| examples/perl/cpanm-api-integration/scripts/cpanfile | Adds a cpanfile for project dependency installation. |
| examples/perl/cpanm-api-integration/scripts/API.pl | Adds /cert-env endpoint for validating cert env propagation. |
| examples/perl/cpanm-api-integration/README.md | Removes the example README. |
| examples/perl/cpanm-api-integration/CpanmApiIntegration.AppHost.TypeScript/tsconfig.json | New TS AppHost TypeScript configuration. |
| examples/perl/cpanm-api-integration/CpanmApiIntegration.AppHost.TypeScript/package.json | New TS AppHost package manifest (dev tooling + deps). |
| examples/perl/cpanm-api-integration/CpanmApiIntegration.AppHost.TypeScript/package-lock.json | Lockfile for deterministic installs in CI. |
| examples/perl/cpanm-api-integration/CpanmApiIntegration.AppHost.TypeScript/eslint.config.mjs | Adds lint configuration for the TS AppHost. |
| examples/perl/cpanm-api-integration/CpanmApiIntegration.AppHost.TypeScript/aspire.config.json | Adds TypeScript AppHost config and package-to-project mapping for local integration. |
| examples/perl/cpanm-api-integration/CpanmApiIntegration.AppHost.TypeScript/apphost.ts | New TS AppHost using the exported Perl APIs and cert trust configuration. |
| eng/testing/validate-typescript-apphost.ps1 | Enhances validation script to support configured packages, skip-start, and HTTP probing. |
| .gitignore | Ignores generated .aspire/ artifacts under TS AppHost directories. |
Files not reviewed (1)
- examples/perl/cpanm-api-integration/CpanmApiIntegration.AppHost.TypeScript/package-lock.json: Language not supported
aaronpowell
left a comment
There was a problem hiding this comment.
I have some issues with this PR:
- Are we currently not setting up the appropriate Perl toolchain before the tests using the C# AppHost? If we aren't, then I'd suggest that we're lacking test coverage that should be addressed separately
- I'm not sure I understand the motivation behind the overhaul of the PowerShell script for running the TypeScript AppHost tests - are there specific things in the way that the current script operations that results in gaps in what's needed for the Perl integration to run in TypeScript, or are you trying to add resiliency (or similar)? If it's the latter, let's split that into a separate PR so that we can start having more focused PRs
- There are some other tests that aren't TypeScript related - again let's try to split them into separate PRs rather than have PRs that are addressing multiple things at once - that makes them take more time to review
I do not think this suggests we're lacking test coverage, the unit tests cover this behavior. I originally submitted 6 examples to integration test so I could integration test everything. We pared that back to one example so I chose to show a simple C# apphost example with CPANM as the package manager. I thought it might be good - since we need to put a typescript apphost in to an integration test anyways - to diversify what we're integration testing since I didn't overload my one example with every single feature available (competing package managers like Carton, for example). I have converted the Typescript example/integration test to be a carbon copy of the C# apphost which reduces the need to make the test runner have Carton installed. Counter Question: It seems like I should probably over time expand my integration test to make the singular integration test cover every behavior?
This was actually a rabbit hole I went down that I believe was tied to the Typescript aspire.config.json having a "channel: stable" in it that prevented it from pulling the polyglot.local package from local. I have reverted it.
I have pulled out the tests that weren't related to the narrow Typescript changes. In general I am hearing that the tighter the PR the easier the review, I will consider that as I go forward. For the ps1 change, it was to put a bandaid over something screwy with my aspire.config.json for my Typescript apphost. I think what happened was - the first time it runs, if it has "Channel: stable" it can't get the polyglot.local package, but I removed the channel stable as a test and it worked, then put it back and it worked... again. I think because it is now cached so the restore is fine. I will want more time to revisit as I think there's still something to tease out there and it's very late so I'm going to revisit another night. |
Will expand more before PR.
…stion about if these validate-typescript-apphost.ps1 changes are required. Will interrogate later.
The offense is that "channel: stable" is blocking the polyglot.local packages from being picked up _UNLESS_ you have already built it and it's in your cache. This sent me down a rabbit hole that made me work around it with the powershell tweaks.
…ith the current state of .mts Typescript implementations. I am less confident about stripping down the AspireExport() and removing the description, but I believe Seb did this as part of PR 1370.
f8a5165 to
1578403
Compare
…ay back in. These are no longer needed simply because I stopped using "channel: stable" in the aspire.config.json.
I believe this removes the diff hit, I removed the newline after the }
|
Alright, I believe this is ready for re-review, but I will note that my rebase I messed up the typescript ps1, and had to stair step those back out. I am not above doing a new PR and removing the messy commit history here if that's something of concern. I also - because it was strictly related to typescript - went ahead and made the change to the .mts and to leverage the work of @sebastienros from #1370 Let me know if you don't like anything here and I'll make it right. |
|
I see that my one Typescript apphost test failed. It's this line causing it from Main, I believe. It's affecting at least the Java integration as well. Line 16 in 5e651f6 |
**Closes #1361 **
Added typescript support by attempting to follow the guide here https://aspire.dev/extensibility/multi-language-integration-authoring/
PR Checklist
Other information
Open to feedback.