-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[attestation] Migrate to ESM and tshy #31433
Open
maorleger
wants to merge
15
commits into
Azure:main
Choose a base branch
from
maorleger:attestation-esm
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 14 commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
97bd2c3
Migration: Update package.json, tsconfig.json, and api-extractor.json
maorleger 62bdfe0
Migration: Update test config
maorleger 1edaf03
Migration: Clean up files
maorleger 58d1d8d
Migration: Apply codemod: "fixSourceFile"
maorleger 2e538a3
Migration: Apply codemod: "fixTestingImports"
maorleger 7d1e873
Migration: Apply codemod: "replaceAssertIsRejected"
maorleger dfefc95
Migration: Apply codemod: "replaceSinonStub"
maorleger c3d0fa2
Migration: Apply codemod: "addViHelper"
maorleger 8499570
Migration: Apply codemod: "replaceSupportTracing"
maorleger 16b15d9
Migration: Apply codemod: "replaceTestUtils"
maorleger 09ee4e9
Migration: rushx format
maorleger 3b7eb41
rush update and build
maorleger 51d0b26
post migration fixes
maorleger d827766
final fixes
maorleger eb98803
rush update and build
maorleger File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Would we assume the test-proxy is required for unit tests as well as for integration tests?
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.
I think the
no-test-proxy
flag should be reserved for packages that don’t use recordings at all, but I could be overlooking something.Currently, the
TEST_MODE
environment variable seems to guide whether we use the proxy or not:However, the challenge I see is the distinction between
unit-test
andintegration-test
. This split seems to come from when we used to rununit-test
from source code andintegration-test
from the distribution (JavaScript). But now that we’ve consolidated everything under Vitest, it raises the question: shouldintegration-test
only be used for live tests now? Or should we just rely onTEST_MODE
entirely?Another option could be to remove the
integration-test
script altogether, as runningTEST_MODE=live rushx unit-test
would achieve the same result as the currentintegration-test
. This might be something worth discussing with the team, as the purpose behind keeping both scripts isn't entirely clear to me anymore.idk, what do folks think?