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

feat(azure-end-to-end-tests): Expand AzureClient e2e tests #21041

Merged
merged 12 commits into from
May 16, 2024

Conversation

scottn12
Copy link
Contributor

@scottn12 scottn12 commented May 10, 2024

Description

This PR updates @fluidframework/azure-end-to-end-tests to import legacy (LTS) AzureClient APIs. This will allow us to test collaboration between legacy and current AzureClient users.

Additionally, this PR adds AzureClient e2e tests (against tinylicious) to the build pipeline for PRs. This will require AzureClient tests to pass to check-in future PRs.

Reviewer Guidance

  • Additional 1.x/2.x compatibility smoke tests that you would like to see added in this or a future PR
  • Thoughts on the method I used to have Azure e2e tests ran in build pipelines for PRs (see the logs for this PR as an example).
  • Thoughts on importing the legacy AzureClient/SharedMap APIs directly in package.json. The alternative is to setup a programmatic way to download these legacy packages (more similar to the setup in @fluid-private/test-end-to-end-tests)

Misc

One-pager for context
AB#7690

@github-actions github-actions bot added area: build Build related issues area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct dependencies Pull requests that update a dependency file base: main PRs targeted against main branch labels May 10, 2024
@scottn12 scottn12 marked this pull request as ready for review May 13, 2024 15:56
Copy link
Contributor

@ChumpChief ChumpChief left a comment

Choose a reason for hiding this comment

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

Generally looks good to me! Others might have better insight on the "how we run our tests in the pipeline questions" but overall looks like what I'd expect.

@@ -26,6 +26,7 @@
"lint:fix": "fluid-build . --task eslint:fix --task format",
"start:tinylicious:test": "PORT=7071 tinylicious > tinylicious.log 2>&1",
"test": "npm run test:realsvc",
"test:azureclient:tinylicious:report": "npm run test:realsvc:tinylicious",
Copy link
Contributor

Choose a reason for hiding this comment

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

I see this is doing the same thing as test:realsvc, which is already run by test - maybe don't need the duplication?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea was to give it a unique name so that when we run it in the build pipeline it is obvious what we are running, see here

Copy link
Contributor

Choose a reason for hiding this comment

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

Your call (or maybe someone who thinks more about test scripts will have some opinions) - I don't have a strong opinion here, so just presenting some alternative ideas.

"@fluidframework/container-definitions": "workspace:~",
"@fluidframework/container-loader": "workspace:~",
"@fluidframework/core-interfaces": "workspace:~",
"@fluidframework/counter": "workspace:~",
"@fluidframework/datastore-definitions": "workspace:~",
"@fluidframework/fluid-static": "workspace:~",
"@fluidframework/map": "workspace:~",
"@fluidframework/map-legacy": "npm:@fluidframework/map@^1.4.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO this is prob fine, at least for now. Theoretically this means we need to be diligent about updating the import as we patch 1.x, but I'd defer engineering a solution unless/until we determine that's a frequent enough occurrence that it needs something automated.

@@ -38,6 +38,7 @@
"checks:fix": "fluid-build --task checks:fix",
"ci:build": "fluid-build --task ci:build",
"ci:build:docs": "fluid-build --task ci:build:docs",
"ci:test:azureclient:tinylicious": "pnpm run -r --no-sort --stream --no-bail test:azureclient:tinylicious:report",
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than running this recursively, maybe target it specifically at the azure client end-to-end tests and the command you want?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that makes sense, but I was aiming to replicate how we run our other e2e test suite for consistency

@scottn12 scottn12 requested a review from ChumpChief May 15, 2024 18:48
Copy link
Contributor

@ChumpChief ChumpChief 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!

clientCurrent = createAzureClient();
clientLegacy = createAzureClientLegacy();
if (isEphemeral) {
// TODO: Should we skip ephemeral tests for legacy clients?
Copy link
Contributor

Choose a reason for hiding this comment

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

I think skipping is probably fine here for now. Longer-term, when we add ephemeral support to AzureClient we'll probably want to cover it, but since it's not a public API currently I think we can skip.

@msfluid-bot
Copy link
Collaborator

@fluid-example/bundle-size-tests: -486 Bytes
Metric NameBaseline SizeCompare SizeSize Diff
aqueduct.js 453.18 KB 453.18 KB No change
azureClient.js 550.61 KB 550.39 KB -220 Bytes
connectionState.js 680 Bytes 680 Bytes No change
containerRuntime.js 257.02 KB 257.02 KB No change
fluidFramework.js 357.56 KB 357.54 KB -22 Bytes
loader.js 132.89 KB 132.89 KB No change
map.js 41.45 KB 41.45 KB No change
matrix.js 143.67 KB 143.67 KB No change
odspClient.js 518.94 KB 518.94 KB No change
odspDriver.js 97.29 KB 97.29 KB No change
odspPrefetchSnapshot.js 42.15 KB 42.15 KB No change
sharedString.js 160.19 KB 160.19 KB No change
sharedTree.js 357.55 KB 357.53 KB -22 Bytes
Total Size 3.19 MB 3.19 MB -486 Bytes

Baseline commit: 3f33cdb

Generated by 🚫 dangerJS against b91f575

@scottn12 scottn12 merged commit c850460 into microsoft:main May 16, 2024
30 checks passed
@scottn12 scottn12 deleted the expandAzureClientE2eTests branch May 16, 2024 13:25
yann-achard-MS added a commit to yann-achard-MS/FluidFramework that referenced this pull request May 16, 2024
kekachmar pushed a commit to kekachmar/FluidFramework that referenced this pull request May 21, 2024
…#21041)

## Description

This PR updates `@fluidframework/azure-end-to-end-tests` to import
legacy (LTS) AzureClient APIs. This will allow us to test collaboration
between legacy and current AzureClient users.

Additionally, this PR adds AzureClient e2e tests (against tinylicious)
to the build pipeline for PRs. This will require AzureClient tests to
pass to check-in future PRs.

## Reviewer Guidance
- Additional 1.x/2.x compatibility smoke tests that you would like to
see added in this or a future PR
- Thoughts on the method I used to have Azure e2e tests ran in build
pipelines for PRs (see the
[logs](https://dev.azure.com/fluidframework/public/_build/results?buildId=263659&view=logs&j=3dc8fd7e-4368-5a92-293e-d53cefc8c4b3&t=ddd9efee-bc1d-59c0-82a9-543538b126ce)
for this PR as an example).
- Thoughts on importing the legacy AzureClient/SharedMap APIs directly
in package.json. The alternative is to setup a programmatic way to
download these legacy packages (more similar to the setup in
`@fluid-private/test-end-to-end-tests`)

## Misc

[One-pager](https://microsoft.sharepoint-df.com/:fl:/g/contentstorage/CSP_5fcca107-c97c-4b0a-899e-42a2578d1609/EX3Cew9vjshMmgeYPckgHHkB7O_6yukNZg00D2mrpR2kMA?e=ZBWefW&nav=cz0lMkZjb250ZW50c3RvcmFnZSUyRkNTUF81ZmNjYTEwNy1jOTdjLTRiMGEtODk5ZS00MmEyNTc4ZDE2MDkmZD1iJTIxQjZITVgzekpDa3VKbmtLaVY0MFdDY3JoRTNub2dqNURqZ01yWHlBTGQ3MWxiZ0VwY3VvYlFKXzhnb2luWjJFTyZmPTAxTUdUSktNTDVZSjVRNjM0T1pCR0pVQjRZSFhFU0FIRFomYz0lMkYmYT1Mb29wQXBwJnA9JTQwZmx1aWR4JTJGbG9vcC1wYWdlLWNvbnRhaW5lciZ4PSU3QiUyMnclMjIlM0ElMjJUMFJUVUh4dGFXTnliM052Wm5RdWMyaGhjbVZ3YjJsdWRDMWtaaTVqYjIxOFlpRkNOa2hOV0RONlNrTnJkVXB1YTB0cFZqUXdWME5qY21oRk0yNXZaMm8xUkdwblRYSlllVUZNWkRjeGJHSm5SWEJqZFc5aVVVcGZPR2R2YVc1YU1rVlBmREF4VFVkVVNrdE5UMFZDTWxGRFRVSlZSMVpDUVV4R1RGVkpNelpOTlVaT1JqTSUzRCUyMiUyQyUyMmklMjIlM0ElMjJhNDE5Zjk2MS1mNDA2LTQ5MmEtYmZjNi0yZTFmYTFiYzIyNjklMjIlN0Q%3D)
for context

[AB#7690](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/7690)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: build Build related issues area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct base: main PRs targeted against main branch dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants