playwright icon indicating copy to clipboard operation
playwright copied to clipboard

[Feature]: Removal of `mount()` inside `test.beforeEach()` and `test.afterAll()`

Open sand4rt opened this issue 1 year ago • 6 comments

🚀 Feature Request

The removal of mount() inside test.beforeEach() and test.afterAll()

Example

This pattern:

const test = baseTest.extend({
  mount({ mount }, use) {
    return use((props) => mount(<Context><Button title="default_title" {...props} /></Context>));
  }
});

test('ok', async ({ mount }) => {
  const component = await mount({ title: 'override_title' });
  await expect(component).toContainText('override_title');
});

Results in a way simpler testbase compared to:

let component;
let props = {};
test.beforeEach(async ({ mount }) => {
  component = await mount(<Context><Button title="default_title" {...props} /></Context>);
  props = {};
});

test('bad', async () => {
  props = { title: 'override_title' };
  await expect(component).toContainText('override_title');
});

Motivation

I've often seen that the use of test.beforeEach and test.afterEach in component testing creates an unmaintainable mess. Kent C. Dodds also discussed this in a blog post and introduced an ESLint rule to address this issue.

One of the great advantages of Playwright fixtures is how they can prevent this issue, so there is no need for an ESLint rule.

sand4rt avatar Jun 17 '24 17:06 sand4rt

Welcome to LocalStack! Thanks for reporting your first issue and our team will be working towards fixing the issue for you or reach out for more background information. We recommend joining our Slack Community for real-time help and drop a message to LocalStack Pro Support if you are a Pro user! If you are willing to contribute towards fixing this issue, please have a look at our contributing guidelines and our contributing guide.

localstack-bot avatar Oct 16 '23 21:10 localstack-bot

Hello @elliotvargas and thanks for your report!

I can confirm the issue: the template is applied, but we don't take context.requestOverride.path into consideration at the moment. I'm currently on something regarding the APIGateway -> S3Integration, and I'll try to add this feature as well, I have something experimental now that makes your sample work.

$ curl http://localhost:4566/restapis/test-api/local/_user_request_/v1/2/2
{
  "id": 2,
  "name": "Document Two"
}

I hope I can get this ready soon, I'll keep you informed of the progress here. Thanks again!

bentsku avatar Oct 16 '23 22:10 bentsku

Hello 👋! It looks like this issue hasn’t been active in longer than five months. We encourage you to check if this is still an issue in the latest release. In the absence of more information, we will be closing this issue soon. If you find that this is still a problem, please feel free to provide a comment or upvote with a reaction on the initial post to prevent automatic closure. If the issue is already closed, please feel free to open a new one.

localstack-bot avatar Jun 06 '24 14:06 localstack-bot

Hello @elliotvargas and thanks again for the report.

We're currently in the process of rewriting our API Gateway implementation, and REST API are now available in the latest and 3.7 version. We've entirely reworked it, and this should now be fixed. Could you please pull the latest version or 3.7 of LocalStack with docker pull localstack/localstack:3.7 and try to start LocalStack with the following environment variable PROVIDER_OVERRIDE_APIGATEWAY=next_gen ?

Thanks again and please feel free if there's anything!

bentsku avatar Sep 03 '24 16:09 bentsku

Hello 👋! It looks like this issue hasn’t been active in longer than two weeks. We encourage you to check if this is still an issue in the latest release. In the absence of more information, we will be closing this issue soon. If you find that this is still a problem, please feel free to provide a comment or upvote with a reaction on the initial post to prevent automatic closure. If the issue is already closed, please feel free to open a new one.

localstack-bot avatar Oct 16 '24 14:10 localstack-bot

Hello again! The new API Gateway provider has been officially released, you can read more about it in our release announcement for 3.8: https://blog.localstack.cloud/localstack-release-v-3-8-0/#new-api-gateway-provider

This issue should now be fixed by using the feature flag. Thank you!

bentsku avatar Oct 16 '24 19:10 bentsku