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: Publicly Expose fixture<T> in @microsoft/fast-tooling #4930

Closed
RehanSaeed opened this issue Jul 1, 2021 · 16 comments · Fixed by #5958
Closed

feat: Publicly Expose fixture<T> in @microsoft/fast-tooling #4930

RehanSaeed opened this issue Jul 1, 2021 · 16 comments · Fixed by #5958
Assignees
Labels
area:fast-foundation Pertains to fast-foundation closed:done Work is finished community:request Issues specifically reported by a member of the community. feature A new feature

Comments

@RehanSaeed
Copy link

🙋 Feature Request

@microsoft/fast-tooling has fixture<T> which seems like a really useful helper function for tests. It's not currently publicly exposed, can that be changed?

🤔 Expected Behavior

Allow us to import fixture<T>.

😯 Current Behavior

fixture<T> is not publicly exposed, so can't import it.

💁 Possible Solution

Add it to the @microsoft/fast-tooling package.

@RehanSaeed RehanSaeed added the status:triage New Issue - needs triage label Jul 1, 2021
@EisenbergEffect EisenbergEffect self-assigned this Jul 7, 2021
@EisenbergEffect EisenbergEffect added area:fast-foundation Pertains to fast-foundation community:request Issues specifically reported by a member of the community. feature A new feature status:under-consideration Issue is being reviewed by the team. and removed status:triage New Issue - needs triage labels Jul 7, 2021
@EisenbergEffect
Copy link
Contributor

EisenbergEffect commented Jul 7, 2021

@RehanSaeed I've been thinking of creating a new @microsoft/fast-testing package which could contain this and other helpers present and future. What do you think of that idea?

There's some trick with circular dependencies. For example, the helpers leverage foundation but are also needed to test foundation. If put into another package, I'm not sure how that would work exactly.

@RehanSaeed
Copy link
Author

@RehanSaeed I've been thinking of creating a new @microsoft/fast-testing package which could contain this and other helpers present and future. What do you think of that idea?

Seems like a good idea.

@rajsite
Copy link

rajsite commented Jul 23, 2021

+1 we're trying to leverage it for testing our components as well.

While you're digging in there could the type for Fixture be updated from:

export interface Fixture<TElement = HTMLElement> {
    // ...
    connect(): Promise<void>;
    disconnect(): Promise<void>;
}

to:

export interface Fixture<TElement = HTMLElement> {
    // ...
    connect: () => Promise<void>;
    disconnect: () => Promise<void>;
}

The method type worries the unbound-method eslint rule because it thinks we are destructuring a method and invoking it as a function with the following pattern:

const { element, connect, disconnect } = await fixture('my-element');
await connect();

the workaround to settle eslint's nerves is to invoke it like a method as the type currently suggests:

const myFixture = await fixture('my-element');
await myFixture.connect();

@EisenbergEffect
Copy link
Contributor

I should be able to get the types fixed up fairly quickly. Let me do that first and then we can revisit the topic of how to make this more broadly available.

@EisenbergEffect
Copy link
Contributor

I've got a PR up to adjust the types: #5016

@EisenbergEffect
Copy link
Contributor

Ok, PR is merged. So, types should be updated in the next release (I believe tonight).

We're having a discussion on how we extract this for proper use by the community. I think there's a bit more involved due to how this affects package dependencies in our monorepo. For some more details on that, I've written up a brief explanation in the above PR's "Next Steps" section. There's some discussion ongoing. I hope we'll be able to address this but it may take some time due to related monorepo work.

@rajsite
Copy link

rajsite commented Aug 2, 2021

I recently became aware of the @open-wc/testing package. Interesting to see a take on a web-component agnostic set of test fixture tools. Could be an interesting compare point for what features to put in a separate package / another data point for having a separate package.

@EisenbergEffect
Copy link
Contributor

Looking back into this, I think it probably makes sense to keep the testing helpers with the foundation library since they are both used to test foundation and also use some of the types from foundation. My thought was that we could create a testing module at the root that would export all test related functionality so that in tests folks could import it by path. For example, something like this:

import { uniqueElementName, fixture } from '@microsoft/fast-foundation/src/testing'

Maybe we could use some new Node package features to make this nicer as well.

When I tried the basic approach out here I found that the TS autocomplete and build worked but the module could not be found at runtime. When I switched it to using the esm source from the dist folder, it would build and work fine at runtime, but TS couldn't provide any type checking or autocomplete. Ideally we would get all the benefits of TS and of course have the module import properly.

I'm curious if anyone has experience taking this approach. I'm wondering if the issues I faced are a symptom of our build setup and not so much the approach. Or even if I just don't know what I'm doing or messed up some configuration 🤣

Feedback welcome.

@stale
Copy link

stale bot commented Apr 16, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the warning:stale No recent activity within a reasonable amount of time label Apr 16, 2022
@rajsite
Copy link

rajsite commented Apr 18, 2022

We'd still be interested in having the fixture utility available in some form so we can share test patterns with FAST

@stale stale bot removed the warning:stale No recent activity within a reasonable amount of time label Apr 18, 2022
@EisenbergEffect EisenbergEffect added this to the FASTElement 2.0 milestone Apr 18, 2022
@EisenbergEffect EisenbergEffect moved this from Triage to Backlog in FAST Architecture Roadmap Apr 18, 2022
@EisenbergEffect
Copy link
Contributor

Planning this for the set of releases based on FASTElement 2.0.

@EisenbergEffect EisenbergEffect moved this from Backlog to Todo in FAST Architecture Roadmap Apr 18, 2022
@EisenbergEffect EisenbergEffect moved this from Todo to In Progress in FAST Architecture Roadmap Apr 21, 2022
@EisenbergEffect EisenbergEffect linked a pull request Apr 22, 2022 that will close this issue
5 tasks
@EisenbergEffect EisenbergEffect added status:in-progress Work is in progress and removed status:under-consideration Issue is being reviewed by the team. labels Apr 22, 2022
@EisenbergEffect
Copy link
Contributor

PR under review to add this as part of the next major release.

@EisenbergEffect EisenbergEffect moved this from In Progress to In Review in FAST Architecture Roadmap Apr 22, 2022
@rajsite
Copy link

rajsite commented Apr 22, 2022

@EisenbergEffect it looks like FASTElement 2 is getting close, will there be -beta or -rc pre-release packages released to test against before the 2.0 release?

@EisenbergEffect
Copy link
Contributor

We've got to get together and figure out the logistics of that. I'd like there to be an official beta, rc, etc. yes.

@rajsite
Copy link

rajsite commented Apr 22, 2022

Sounds great! 🎉

@EisenbergEffect EisenbergEffect linked a pull request May 10, 2022 that will close this issue
9 tasks
@EisenbergEffect EisenbergEffect added closed:done Work is finished and removed status:in-progress Work is in progress labels May 11, 2022
@EisenbergEffect
Copy link
Contributor

Betas coming very soon 😄

Repository owner moved this from In Review to Done in FAST Architecture Roadmap May 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:fast-foundation Pertains to fast-foundation closed:done Work is finished community:request Issues specifically reported by a member of the community. feature A new feature
Projects
Archived in project
4 participants