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

Provide different layers' APIs to describeCompat suites #15917

Merged
merged 17 commits into from
Jun 19, 2023

Conversation

Abe27342
Copy link
Contributor

@Abe27342 Abe27342 commented Jun 8, 2023

Description

This allows e2e tests which create their own data objects to correctly test the same compatibility matrix that other e2e tests do. See #12052 for a historical approach which generally aligns with this one.

Note: CI won't pass on this as I haven't bothered updating the 3 example packages that use test-version-utils.

@github-actions github-actions bot added area: tests Tests to add, test infrastructure improvements, etc base: main PRs targeted against main branch labels Jun 8, 2023
@Abe27342
Copy link
Contributor Author

Abe27342 commented Jun 8, 2023

This is a draft because if we want to go through with this technique, I'll split out the ESNext module changes into their own PR so this isn't so ridiculous to review.

Some alternative options we have are:

  1. Implement basically analogous approach to Added options in TestObjectProvider to return compat versions of container runtime and data store runtime #12052, where we accept the fact that people can't make their data objects until test hooks / tests (I'd really rather not do this)
  2. Download compat matrix in a separate process run before the test process (I'd be more ok with this, but I still like the current approach more)
  3. Engage more with mocha folks to see likelihood of the global test fixture issue being fixed (🚀 Feature: Global fixtures should run before any files are loaded mochajs/mocha#4508)
  4. use execSync commands everywhere we install packages (doesn't seem great for perf)

@github-actions github-actions bot added the area: examples Changes that focus on our examples label Jun 9, 2023
@github-actions github-actions bot added the dependencies Pull requests that update a dependency file label Jun 9, 2023
@Abe27342
Copy link
Contributor Author

Abe27342 commented Jun 9, 2023

This generally works. I need to resolve a build error that looks like it has to do with parallelization of compatibility package installation so CI fails on the overall PR, but that doesn't need to be addressed until test-version-utils is converted to ESNext. I'm therefore going to start opening PRs to convert packages depending on test-version-utils to ESNext, before addressing that.

Abe27342 added a commit that referenced this pull request Jun 12, 2023
## Description

This makes progress toward #15917 by deleting test-gc-sweep-tests, which
is not needed anymore as per @agarwal-navin.

PACKAGES.md was updated using `flub check layers --info
./build-tools/packages/build-tools/data/layerInfo.json --md .`.
@Abe27342
Copy link
Contributor Author

This generally works. I need to resolve a build error that looks like it has to do with parallelization of compatibility package installation so CI fails on the overall PR, but that doesn't need to be addressed until test-version-utils is converted to ESNext. I'm therefore going to start opening PRs to convert packages depending on test-version-utils to ESNext, before addressing that.

The issue in CI isn't actually a race condition at all. The NODE_OPTIONS I used to inject ./ignore-loader.mjs are persisted for a nested npm command in webflow, which isn't able to resolve the import. In fact, using esm-loader-css doesn't work either due to pnpm's restrictive install tree. I'm planning on fixing this by just resetting NODE_OPTIONS in the npm commands in test-version-utils.

Abe27342 added a commit that referenced this pull request Jun 12, 2023
…5939)

## Description

This makes progress toward #15917 by converting
`@fluid-example/table-document`'s tests to run esnext modules only.

It's not yet feasible to convert the whole package to esnext, as despite
being an example package it's currently used by some internal partners.
I've therefore just converted the test config to use esnext.
Abe27342 added a commit that referenced this pull request Jun 15, 2023
## Description

Transitions `@fluid-internal/test-end-to-end-tests` to ESNext modules. 

This makes progress toward #15917, a planned improvement for
`describeCompat`.
Abe27342 added a commit that referenced this pull request Jun 16, 2023
## Description

This makes progress toward #15917 by converting @fluid-example/webflow
to use esnext modules.

This is the most involved update of any of the packages that depend on
`@fluid-internal/test-version-utils`, since ESNext modules don't support
the `require.extensions` api (they don't support `require`), which was
the technique used to make webpack-style CSS imports not blow up when
run in a node environment. Instead, it's recommended to use the node esm
loader API. Although this API is experimental, it hasn't receive any
changes between v16.12 and the node latest (v20.x).
@github-actions github-actions bot removed the dependencies Pull requests that update a dependency file label Jun 16, 2023
@github-actions github-actions bot removed the area: examples Changes that focus on our examples label Jun 16, 2023
@Abe27342 Abe27342 marked this pull request as ready for review June 16, 2023 17:17
@Abe27342 Abe27342 requested review from msfluid-bot and a team as code owners June 16, 2023 17:17
@Abe27342
Copy link
Contributor Author

@agarwal-navin I think this is ready to go once our PR builds are healthy again (it passed build/test step, so rest of the pipeline should be green).

@justus-camp if you could review general esnext module changes here (and feel free to review the compat testing api changes, they don't require much context to understand), i'd love to get this in today or early next week :)

@Abe27342 Abe27342 requested a review from justus-camp June 16, 2023 21:01
@msfluid-bot
Copy link
Collaborator

@fluid-example/bundle-size-tests: +30 Bytes
Metric NameBaseline SizeCompare SizeSize Diff
aqueduct.js 448.33 KB 448.34 KB +6 Bytes
connectionState.js 680 Bytes 680 Bytes No change
containerRuntime.js 239.04 KB 239.04 KB +2 Bytes
loader.js 155.61 KB 155.62 KB +4 Bytes
map.js 46.75 KB 46.75 KB +2 Bytes
matrix.js 146.68 KB 146.68 KB +2 Bytes
odspDriver.js 92.44 KB 92.44 KB +6 Bytes
odspPrefetchSnapshot.js 43.69 KB 43.69 KB +4 Bytes
sharedString.js 163.28 KB 163.29 KB +2 Bytes
sharedTree2.js 248.74 KB 248.74 KB No change
Total Size 1.7 MB 1.7 MB +30 Bytes

Baseline commit: 54cc669

Generated by 🚫 dangerJS against cbabb42

@Abe27342 Abe27342 merged commit 2c9f2bf into microsoft:main Jun 19, 2023
@Abe27342 Abe27342 deleted the provide-apis-in-describe-compat branch June 19, 2023 16:50
@Abe27342
Copy link
Contributor Author

ah. I never updated the description on this :(. It's accurate besides the note at the end :P

Abe27342 added a commit to Abe27342/FluidFramework that referenced this pull request Jun 21, 2023
Abe27342 added a commit that referenced this pull request Jun 22, 2023
## Description

This allows e2e tests which create their own data objects to correctly
test the same compatibility matrix that other e2e tests do. See
#12052 for a historical
approach which generally aligns with this one.

Note: this is a reapplication of the changes in #15917. Reverting this
PR didn't fix the issue, and we have since discovered there was a test
reporting issue (fixed in #16098). The intermittent failures are
therefore likely due to a flaky test, and not changes related to the
esnext switch.
NicholasCouri added a commit that referenced this pull request Jun 23, 2023
## Description

It seems [PR
15917](#15917) missed
some changes on the .mocha config files.
Sample error:
[buildId=169808](https://dev.azure.com/fluidframework/internal/_build/results?buildId=169808&view=results)
connorskees pushed a commit to connorskees/FluidFramework that referenced this pull request Jul 19, 2023
## Description

This makes progress toward microsoft#15917 by deleting test-gc-sweep-tests, which
is not needed anymore as per @agarwal-navin.

PACKAGES.md was updated using `flub check layers --info
./build-tools/packages/build-tools/data/layerInfo.json --md .`.
connorskees pushed a commit to connorskees/FluidFramework that referenced this pull request Jul 19, 2023
…crosoft#15939)

## Description

This makes progress toward microsoft#15917 by converting
`@fluid-example/table-document`'s tests to run esnext modules only.

It's not yet feasible to convert the whole package to esnext, as despite
being an example package it's currently used by some internal partners.
I've therefore just converted the test config to use esnext.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: tests Tests to add, test infrastructure improvements, etc base: main PRs targeted against main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants