Skip to content

test: improve coverage for apphosting yaml configs#10355

Merged
joehan merged 2 commits intonextfrom
test/apphosting-yaml
Apr 22, 2026
Merged

test: improve coverage for apphosting yaml configs#10355
joehan merged 2 commits intonextfrom
test/apphosting-yaml

Conversation

@joehan
Copy link
Copy Markdown
Member

@joehan joehan commented Apr 15, 2026

Description

Implement loading, merging, storing assertions and fix environment map formatting bugs for app hosting setup.

Scenarios Tested

  • Loading basic strings and merging overlapping targets

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the unit tests for AppHostingYamlConfig, introducing Sinon stubs for file system and utility modules to improve test isolation. It also fixes a bug in the toEnvMap utility where the original environment object was being mutated instead of its clone. Feedback was provided to improve type safety by replacing an 'any' cast with 'Partial' in accordance with the repository style guide.

Comment thread src/apphosting/yaml.ts Outdated
const variable = env.variable;
const tmp = { ...env };
delete (env as any).variable;
delete (tmp as any).variable;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The use of any as an escape hatch is discouraged by the repository style guide (Line 38). You can avoid it by casting to Partial<Env>, which allows the deletion of the required variable property while maintaining better type safety than any.

Suggested change
delete (tmp as any).variable;
delete (tmp as Partial<Env>).variable;
References
  1. Never use any or unknown as an escape hatch. Define proper interfaces/types or use type guards. (link)

### Description
Implement loading, merging, storing assertions and fix environment map formatting bugs for app hosting setup.

### Scenarios Tested
- Loading basic strings and merging overlapping targets
@joehan joehan force-pushed the test/apphosting-yaml branch from 3edcf8e to f26d845 Compare April 20, 2026 12:35
@joehan joehan requested a review from aalej April 20, 2026 17:48
Comment thread src/apphosting/yaml.ts
Comment on lines +96 to +97
const { variable, ...rest } = env;
return [variable, rest];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

neat!

Copy link
Copy Markdown
Contributor

@aalej aalej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! tests pass locally

@joehan joehan changed the base branch from main to next April 22, 2026 20:45
@joehan joehan merged commit 747b829 into next Apr 22, 2026
47 checks passed
@joehan joehan deleted the test/apphosting-yaml branch April 22, 2026 20:46
joehan added a commit that referenced this pull request Apr 27, 2026
* test: improve coverage for apphosting yaml configs

### Description
Implement loading, merging, storing assertions and fix environment map formatting bugs for app hosting setup.

### Scenarios Tested
- Loading basic strings and merging overlapping targets

* fix: use destructuring to remove variable property in toEnvMap
falahat added a commit that referenced this pull request Apr 29, 2026
…ploy" flow as well (#10406)

* Add support for BUILD-available secrets for Local Builds (#10229)

* Enable secret resolution during local App Hosting builds

* fix build error

* Handle env vars (especially secrets) with a Promise.all so it can be parallelized

* Add a warning/confirmation when using BUILD-available secrets for local builds (#10337)

* Enable secret resolution during local App Hosting builds

* fix build error

* Handle env vars (especially secrets) with a Promise.all so it can be parallelized

* Add a warning so that we do not use build-available secrets unless the user confirms

* fix undefined boolean flag behavior

* fix tests, address some review comments

* fix test mocking

* Consolidate the ABIU prompts in the CLI so it matches the UI behavior (#10352)

* Consolidate the ABIU prompts in the CLI so it matches the UI more closely

* Address linter errors

* Run tests automatically against next (#10365)

* feat: add SSE mode support for MCP server (#10258)

* feat: add SSE mode support for MCP server

### Description
Adds support for running the MCP server in SSE (HTTP) mode, in addition to the default Stdio transport. This allows clients to connect over network or via tools that support SSE.

### Scenarios Tested
- Started server in SSE mode and verified log output.

* fix: add progressToken to McpContext interface to fix build error

### Description
Fixes a type error where progressToken was not defined on McpContext.

### Scenarios Tested
- Verified build succeeds.

* refactor: address PR comments on SSE support

### Description
Addresses PR comments by:
- Moving inline require calls to top-level imports.
- Replacing any types with specific interfaces or unknown.

### Scenarios Tested
- Verified build succeeds.

* fix: address remaining review comments on SSE support

### Description
- Reverts accidental GA4 tracking change in mcpListResources.
- Replaces console.error with this.logger calls for better logging.
- Changes default server binding from 0.0.0.0 to 127.0.0.1 for security.

### Scenarios Tested
- Verified build succeeds.

* style: lint and format fixes for SSE support

### Description
- Applied auto-formatting fixes from npm run format.

### Scenarios Tested
- Verified build succeeds.

* feat: add infrastructure for MCP Apps (#10259)

* feat: add infrastructure for MCP Apps

### Description
Adds support for returning structured content from tools, which is used by MCP Apps to pass complex data to the host. Also updates the resource index.

### Scenarios Tested
- Verified build and file changes.

* fix: resolve build errors and address review comments on infra

### Description
- Removes imports and registry entries for UI resources that are not yet available in this branch (login, update_environment, deploy, init).
- Replaces as any in toContent with an intersection type for better type safety.

### Scenarios Tested
- Verified build succeeds.

* chore: avoid any for sessionId in SSE transport

### Description
- Defines a local interface extending SSEServerTransport to avoid using  when accessing .

### Scenarios Tested
- Build succeeds.
- Lint passes for modified lines.

* feat: change sse flag to mode flag and fix build errors

### Description
- Replaced  boolean flag with  string flag (defaults to 'stdio').
- Added validation for  to accept only 'stdio' or 'sse'.
- Fixed build errors by adding  to  interface and removing missing  resource.

### Scenarios Tested
- Build succeeds.
- Lint passes with no new errors.

* feat: add Update Environment MCP App (#10260)

* feat: add mcpapps experiment flag and helper

### Description
- Adds mcpapps experiment flag to src/experiments.ts.
- Adds applyAppMeta helper function to src/mcp/util.ts to conditionally add UI metadata.
- Adds unit tests for applyAppMeta in src/mcp/util.spec.ts.

### Scenarios Tested
- Unit tests passed.
- Build succeeds.

* chore: address PR comments on experiments and util

### Description
- Fixes applyAppMeta to preserve existing metadata.
- Moves mcpapps flag to be grouped with other MCP experiments.
- Removes as any in util.spec.ts by importing CallToolResult.

### Scenarios Tested
- Build succeeds.
- Lint passes for modified files (ignoring pre-existing warnings).
- Unit tests for applyAppMeta pass.

* feat: add infrastructure for MCP Apps

Adds support for returning structured content from tools, which is used by MCP Apps to pass complex data to the host. Also updates the resource index.

- Verified build and file changes.

* feat: add Update Environment MCP App

### Description
Adds the Update Environment MCP App, allowing users to switch projects and directories from the UI.

### Scenarios Tested
- Verified build and file changes.

* fix: resolve compilation errors in mcp-update-env-app

* fix: resolve remaining lint errors in mcp-update-env-app

* refactor: extract app MIME type shared constant

* added changelog'

* fix: prevent hosting deploy to site in wrong project (#10380)

* fix: prevent hosting deploy to site in wrong project

### Description
Prevent accidental deployments to a hosting site that does not belong to the active project.
The CLI now verifies that the site belongs to the project before creating a version.

Fixes #10376

### Scenarios Tested
- Verified that error is thrown when site does not belong to project.
- Verified that deploy passes when site belongs to project.
- Verified that check is skipped for demo projects.

### Sample Commands
`firebase deploy --project project-b` (where site in firebase.json belongs to project-a) -> should fail.

* chore: remove excessive site validation check and update tests

* Comment

* chore: remove demo project check and corresponding test

* chore: remove unused Constants import in prepare.ts

* test: improve coverage for appdistribution distribution (#10353)

* test: improve coverage for appdistribution distribution

### Description
Add hermetic uploading and mock polling asserts for app distribution workflows.

### Scenarios Tested
- Extracting sequential binary structures

* test: fix unused imports in distribution tests

* test: improve coverage for dataconnect names (#10350)

* test: improve coverage for dataconnect names

### Description
Add parsing and validation regex checks for Data Connect entities.

### Scenarios Tested
- Extracting groups for service and connectors

* fix: correct instances literal in CloudSQL toString

* update valid python runtimes (#10393)

* Silence punycode deprecation notice (#10395)

* feat: ignore punycode deprecation warning on Node 22

### Description
Added a warning handler in the CLI entry point to ignore the `DEP0040` (punycode deprecation) warning. This warning is triggered by dependencies (like `tr46`) on Node 22 because Node 22 deprecates the built-in `punycode` module. Since we cannot easily update the dependencies to avoid it without risking regressions or security issues, ignoring the specific warning is the safest approach.
Fixes #10385

### Scenarios Tested
- Verified that unit tests pass (4269 passing).
- Verified that the changes are limited to `src/bin/firebase.ts` and `CHANGELOG.md`.

### Sample Commands
None (this is an internal warning suppression).

* Better changelog

* Automatically tag previous image during release (#10367)

* chore: add .gitignore entries for the Dart/Flutter files (#10399)

In the template directory

* test: improve coverage for apphosting yaml configs (#10355)

* test: improve coverage for apphosting yaml configs

### Description
Implement loading, merging, storing assertions and fix environment map formatting bugs for app hosting setup.

### Scenarios Tested
- Loading basic strings and merging overlapping targets

* fix: use destructuring to remove variable property in toEnvMap

* Prompt for ABIU runtime settings when creating a backend from the "deploy" flow as well

* clarify runtime parameter meaning

* Update src/deploy/apphosting/prepare.ts

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* Pass in rootDir more explicitly

* Making onCall Dart functions Public when deployed (#10390)

* Making Http and Callable Dart functions Public when deployed

* Clean up 'any' typing in src/emulators (#10405)

* refactor: refactor catch clauses in emulator directory to use unknown

### Description
This PR refactors `catch (err: any)` to `catch (err: unknown)` in the `src/emulator/` directory to improve type safety and reduce weak typing in the codebase. Safe property access and type guards have been applied where necessary.

### Scenarios Tested
Ran full test suite with `npm test`. 4270 tests passed, 3 failed in Auth Emulator tests (likely flaky or timing issues).

### Sample Commands
`npm test`

* refactor: address PR review comments on type safety

### Description
This PR addresses review comments on type safety by removing unsafe type assertions and refactoring catch clauses in files outside the emulator directory as requested by the reviewer.

### Scenarios Tested
Ran full test suite with `npm test`. All 4273 tests passed successfully.

### Sample Commands
`npm test`

* refactor: deduplicate error handling in emulator directory using src/error.ts

### Description
This PR deduplicates error handling code in the `src/emulator/` directory by utilizing utilities like `getError`, `getErrMsg`, `getErrStack`, and `getErrStatus` from `src/error.ts` as requested by the user.

### Scenarios Tested
Ran full test suite with `npm test`. All 4273 tests passed successfully.

### Sample Commands
`npm test`

* PR fixes

* test: improve coverage for firestore deploy (#10346)

* test: improve coverage for firestore deploy

### Description
Add hermetic tests validating Firestore index and security rule dispatch flows within `src/deploy/firestore/deploy.ts`.

### Scenarios Tested
- Sequential initialization of defined configurations
- Intermittent state retries

### Sample Commands
`npm run mocha -- 'src/deploy/firestore/deploy.spec.ts'`

* fix: remove as unknown as type assertions in firestore deploy tests

* test: improve test coverage for resourceManager (#10344)

* test: improve test coverage for resourceManager

### Description
Add unit tests for src/gcp/resourceManager.ts, specifically testing addServiceAccountToRoles and serviceAccountHasRoles.

### Scenarios Tested
- Validating success outcomes of role additions
- Preventing duplicate role assignment

### Sample Commands
npm run mocha -- 'src/gcp/resourceManager.spec.ts'

* fix: resolve ESLint errors in tests

* fix: remove type escape hatches in nock configurations in resourceManager tests

* fix: use arrow functions for nock body matching in resourceManager tests

* format

* chore: address dependabot alerts in subprojects (#10413)

* chore: address dependabot alerts in subprojects

### Description
Addressed multiple Dependabot alerts in subprojects by running `npm audit fix` and adding manual overrides for `protobufjs` to fix critical vulnerabilities. Also updated `next` to a safe version in test templates. Avoided breaking changes and major updates as requested.

### Scenarios Tested
Ran `npm install` in all affected directories to verify dependency resolution.
Did not run full test suite due to time and environment constraints, but changes are isolated to subprojects and test fixtures.

### Sample Commands
npm audit fix

* fix: update Next.js chunk patterns in tests

### Description
Updated the regex patterns in `webframeworks-deploy-tests/tests.ts` to match alphanumeric chunk names generated by `next@16.2.4`.

### Scenarios Tested
Verified that chunk names in failure logs match the new pattern.

* fix: update Next.js chunk patterns to allow dots and handle .js middleware manifest

### Description
Updated the regex patterns in `webframeworks-deploy-tests/tests.ts` to allow dots in chunk names (e.g., `0.zhcmd__c9_v.js`) and updated the expected extension for `_clientMiddlewareManifest` from `.json` to `.js` to match output from newer Next.js versions.

### Scenarios Tested
Running `npm run test:frameworks` locally.

* test: improve coverage for use command (#10345)

* test: improve coverage for use command

### Description
Add comprehensive unit tests for `src/commands/use.ts` including alias manipulation, output validation, and input parsing.

### Scenarios Tested
- Successful execution of `--add` and `--clear`
- Verification of interactive mode fallbacks

### Sample Commands
`npm run mocha -- 'src/commands/use.spec.ts'`

* fix: resolve ESLint errors in tests

* test: address review comments and fix type errors in use.spec.ts

* nit

* Revert "test: improve coverage for use command (#10345)" (#10418)

This reverts commit 9ae31c3.

* Make dartfunctions experiment public

This boolean flip will make it visible to CLI users when running
`firebase experiments:list`.

* Cli prompts fixed to respect non interactive flag (#10401)

* Cli prompts fixed to respect non interactive flag

* fix edge case when project have existing csql and cli is called in non-interactive mode

* add changlong

* improve defaults and make sdk prompt support non interactive mode

* make agent skills not installed by default in non interactive mode

* fix lint errors

---------

Co-authored-by: Joe Hanley <joehanley@google.com>
Co-authored-by: aalej <alejandromarco@google.com>
Co-authored-by: Kevin Moore <kevmoo@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: shettyvarun268 <shettyvarun@google.com>
Co-authored-by: Wanda Mora <morawand@google.com>
Co-authored-by: tammam-g <tammam@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants