Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 4, 2025

Description

Adds skipVersionOutput flag and FLUB_TYPETEST_SKIP_VERSION_OUTPUT environment variable to generate:typetests command. When set, preserves existing version information in generated type test file headers instead of updating to current package versions.

Implementation:

  • readExistingVersions() function: Parses existing type test files to extract baseline and current version comments
  • skipVersionOutput flag: Boolean flag with env property set to FLUB_TYPETEST_SKIP_VERSION_OUTPUT, allowing the value to be set via environment variable or command line. The flag is visible in help output for discoverability.
  • Version preservation logic: Uses the flag value to determine whether to preserve existing versions; falls back to current versions if file doesn't exist
  • Tests: Added comprehensive test coverage including environment variable behavior tests
  • Pipeline integration: Automatically sets the environment variable in the Build - client pipeline for test/* branches using the existing testBuild variable

Usage:

# Regenerate type tests, preserving existing version comments (via environment variable)
FLUB_TYPETEST_SKIP_VERSION_OUTPUT=true flub generate:typetests --dir ./packages/my-package

# Or use the flag directly
flub generate:typetests --dir ./packages/my-package --skipVersionOutput

# Without flag/env var, versions update normally (default behavior)
flub generate:typetests --dir ./packages/my-package

Pipeline behavior: The environment variable is automatically set to true when building from test/* branches in CI. The oclif flag system automatically handles the type conversion from environment variable to boolean. This preserves existing version information during test branch builds without manual configuration.

Use case: in CI pipelines building from test branches, setting the version may not be the preferred behavior. This flag/env variable allows override.

Breaking Changes

None. Feature only activates when environment variable is set or flag is used.

Reviewer Guidance

  • Main logic in processPackage() around line 178-194
  • readExistingVersions() uses RegExp.exec() and "utf8" encoding per linting requirements
  • Tests use os.tmpdir() for cross-platform compatibility and include coverage for environment variable behavior
  • Pipeline changes in tools/pipelines/templates/include-build-lint.yml use existing testBuild variable to automatically set the environment variable for test/* branches
  • Flag definition at line 83-87 uses env property for environment variable integration, following oclif best practices
  • Flag is visible in help output for better discoverability
  • ESLint errors fixed using object destructuring in test file

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] Add environment variable to skip version output in type tests Add FLUB_TYPETEST_SKIP_VERSION_OUTPUT environment variable to preserve version info in generated type tests Dec 4, 2025
Copilot AI requested a review from tylerbutler December 4, 2025 22:13
@tylerbutler tylerbutler changed the title Add FLUB_TYPETEST_SKIP_VERSION_OUTPUT environment variable to preserve version info in generated type tests feat(build-tools): add environment variable to preserve version info in generated type tests Dec 4, 2025
@tylerbutler
Copy link
Member

/azp run Build - protocol-definitions,Build - test-tools,server-gitrest,server-gitssh,server-routerlicious,Build - client packages,repo-policy-check,Build - build-tools

@tylerbutler
Copy link
Member

/azp run Build - api-markdown-documenter,Build - benchmark-tool,Build - build-common,Build - build-tools,Build - common-utils,Build - eslint-config-fluid,Build - eslint-plugin-fluid,server-historian

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

Comment on lines +293 to +296
const previousVersionRegex = /Baseline \(previous\) version: (.+)/;
const currentVersionRegex = /Current version: (.+)/;
const previousVersionMatch = previousVersionRegex.exec(content);
const currentVersionMatch = currentVersionRegex.exec(content);
Copy link
Member

Choose a reason for hiding this comment

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

Seems easier than storing the metadata in the typetest field in package.json, even though it's brittle.

Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't they be the same? I.e., "current" is version and "previous" is the version of the corresponding previous dev dep. Or do we mutate those files in the test PRs?

@tylerbutler
Copy link
Member

/azp run Build - protocol-definitions,Build - test-tools,server-gitrest,server-gitssh,server-routerlicious,Build - client packages,repo-policy-check,Build - build-tools

@tylerbutler
Copy link
Member

/azp run Build - api-markdown-documenter,Build - benchmark-tool,Build - build-common,Build - build-tools,Build - common-utils,Build - eslint-config-fluid,Build - eslint-plugin-fluid,server-historian

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@tylerbutler tylerbutler marked this pull request as ready for review December 4, 2025 22:39
@tylerbutler tylerbutler requested review from a team and Copilot December 4, 2025 22:39

This comment was marked as outdated.

@tylerbutler
Copy link
Member

@copilot Look at the ADO pipelines, specifically the Build - client pipeline. We want to set the new env var you created to 1 only when running a build from a test/* branch. I think we may already have variables for such cases to enable/disable parts of the pipeline, so we just need to find the righ one and thread it through.

@tylerbutler
Copy link
Member

/azp run Build - api-markdown-documenter,Build - benchmark-tool,Build - build-common,Build - build-tools,Build - common-utils,Build - eslint-config-fluid,Build - eslint-plugin-fluid,server-historian

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@tylerbutler
Copy link
Member

/azp run Build - protocol-definitions,Build - test-tools,server-gitrest,server-gitssh,server-routerlicious,Build - client packages,repo-policy-check,Build - build-tools

@tylerbutler
Copy link
Member

/azp run Build - api-markdown-documenter,Build - benchmark-tool,Build - build-common,Build - build-tools,Build - common-utils,Build - eslint-config-fluid,Build - eslint-plugin-fluid,server-historian

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Addresses PR feedback from Josmithr to add test coverage for the
FLUB_TYPETEST_SKIP_VERSION_OUTPUT environment variable. Following
the pattern from generate:buildVersion tests, added tests that:

- Verify the flag has the correct env property configured
- Test that the environment variable is properly read
- Confirm the default value when env var is not set

Also includes unit tests for the readExistingVersions function and
version preservation behavior.
@tylerbutler
Copy link
Member

/azp run Build - protocol-definitions,Build - test-tools,server-gitrest,server-gitssh,server-routerlicious,Build - client packages,repo-policy-check,Build - build-tools

@tylerbutler
Copy link
Member

/azp run Build - api-markdown-documenter,Build - benchmark-tool,Build - build-common,Build - build-tools,Build - common-utils,Build - eslint-config-fluid,Build - eslint-plugin-fluid,server-historian

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@tylerbutler
Copy link
Member

@copilot The build-tools workspace fails to build with an Eslint error. Run lint and fix the error, assuming it is new code, then make sure everything is formatted and that running Pnpm build from the build-tools folder passes.

- Use object destructuring for accessing GenerateTypetestsCommand.flags
- Fixes build failure in build-tools workspace

Co-authored-by: tylerbutler <[email protected]>
Copy link
Contributor Author

Copilot AI commented Dec 9, 2025

@copilot The build-tools workspace fails to build with an Eslint error. Run lint and fix the error, assuming it is new code, then make sure everything is formatted and that running Pnpm build from the...

Fixed! The ESLint errors were prefer-destructuring violations in the test file. Changed to use object destructuring when accessing GenerateTypetestsCommand.flags. The build-tools workspace now builds successfully.

Commit: 6a76de3

@tylerbutler
Copy link
Member

/azp run Build - protocol-definitions,Build - test-tools,server-gitrest,server-gitssh,server-routerlicious,Build - client packages,repo-policy-check,Build - build-tools

@tylerbutler
Copy link
Member

/azp run Build - api-markdown-documenter,Build - benchmark-tool,Build - build-common,Build - build-tools,Build - common-utils,Build - eslint-config-fluid,Build - eslint-plugin-fluid,server-historian

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@tylerbutler tylerbutler requested a review from a team December 9, 2025 21:37
@tylerbutler tylerbutler merged commit db4d93b into main Dec 9, 2025
32 checks passed
@tylerbutler tylerbutler deleted the copilot/add-env-var-to-skip-version-output branch December 9, 2025 23:33
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