Skip to content
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

Update @fluidframework/test-tools in LTS #23449

Merged
merged 3 commits into from
Jan 6, 2025

Conversation

Abe27342
Copy link
Contributor

@Abe27342 Abe27342 commented Jan 3, 2025

Description

  • Updates @fluidframework/test-tools to 2.0.1, using lerna for assign-test-ports
  • Removes the 'label external PRs' GH action from LTS, which is broken and generally doesn't seem like it serves much value in LTS

Fixes issues making LTS pipelines unhealthy.

@msfluid-bot
Copy link
Collaborator

msfluid-bot commented Jan 3, 2025

Could not find a usable baseline build with search starting at CI 9c754ac

Generated by 🚫 dangerJS against 6e087c1

@Abe27342 Abe27342 requested a review from a team as a code owner January 6, 2025 17:35
Copy link
Contributor

@alexvy86 alexvy86 left a comment

Choose a reason for hiding this comment

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

Looks good overall. A few cleanup suggestions but we could merge without them.

Comment on lines +51 to +53
"test:jest": "assign-test-ports --package-manager lerna && lerna run test:jest --concurrency 4 --stream --no-bail --no-sort -- -- --color",
"test:jest:bail": "assign-test-ports --package-manager lerna && lerna run test:jest --concurrency 4 --stream",
"test:jest:report": "assign-test-ports --package-manager lerna && lerna run test:jest --concurrency 4 --stream --no-bail --no-sort -- -- --ci --reporters=default --reporters=jest-junit",
Copy link
Contributor

Choose a reason for hiding this comment

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

build-tools doesn't have jest tests. Maybe I, maybe someone else (can't remember 😄), already removed these scripts and the dependency on test-tools in main, so I'd suggest we keep lts lean and remove them here too.

@@ -44,7 +44,7 @@
"@fluidframework/build-common": "^0.24.0",
"@fluidframework/build-tools": "^0.6.0",
"@fluidframework/eslint-config-fluid": "^0.28.2000",
"@fluidframework/test-tools": "^0.2.3074",
"@fluidframework/test-tools": "^2.0.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems unused, let's remove it?

@@ -46,7 +46,7 @@
"devDependencies": {
"@fluidframework/build-common": "^0.24.0",
"@fluidframework/eslint-config-fluid": "^0.28.2000",
"@fluidframework/test-tools": "^0.2.3074",
"@fluidframework/test-tools": "^2.0.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

This one also seems unused.

@Abe27342
Copy link
Contributor Author

Abe27342 commented Jan 6, 2025

Going to do the removals in follow-ups

@Abe27342 Abe27342 merged commit 3ede640 into lts Jan 6, 2025
16 of 17 checks passed
@Abe27342 Abe27342 deleted the test/absander/update-test-tools-lts branch January 6, 2025 18:18
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