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

Scope testem assets with a prefix #1615

Merged

Conversation

stepankuzmin
Copy link
Contributor

Testem uses an express serve-static middleware to serve the assets. It's mounted on the app root, meaning every request is first processed with the express.static middleware and only after that with the middlewares defined in the config, e.g., proxy middleware.

This ends up in a bunch of extra fs.stat calls which bloat the server response time. You can check that by enabling the DEBUG=send environment variable and making a request to the server.

const testemConfig = {
    middleware: [
        (app) => app.use('/render-tests', serveStatic(path.join(__dirname, 'render-tests')))
    ]
};

Considering the configuration above, all requests with the /render-tests prefix are used for lookup files in the testem/public/ folder first, which should be avoided.

send stat "/mnt/ramdisk/mapbox-gl-js/node_modules/testem/public/render-tests/background-color/default/expected.png" +90ms
send stat "/mnt/ramdisk/mapbox-gl-js/test/integration/render-tests/background-color/default/expected.png" +1ms
send pipe "/mnt/ramdisk/mapbox-gl-js/test/integration/render-tests/background-color/default/expected.png" +0ms

This PR scopes all requests to the /testem prefix to the testem's /public/testem dir, which fixes that issue.

@stepankuzmin
Copy link
Contributor Author

I see some integration tests are failing with GPU process isn't usable, but I'm unable to reproduce it locally with npm run integration. Probably some CI configuration issue?

@johanneswuerbach
Copy link
Member

Could you rebase your PR? CI should be fixed in the latest main :-)

@stepankuzmin
Copy link
Contributor Author

Hey @johanneswuerbach,
Sure, thanks for the fix!

@johanneswuerbach johanneswuerbach merged commit 689efe3 into testem:master Dec 20, 2022
@stepankuzmin stepankuzmin deleted the better-scope-testem-assets branch December 20, 2022 11:28
@johanneswuerbach
Copy link
Member

Published as testem@3.10.1. Thank you 🤩

@stepankuzmin
Copy link
Contributor Author

Awesome, thanks! 🙏

kevinansfield pushed a commit to TryGhost/Ghost that referenced this pull request Jan 3, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [testem](https://togithub.com/testem/testem) | [`3.10.0` ->
`3.10.1`](https://renovatebot.com/diffs/npm/testem/3.10.0/3.10.1) |
[![age](https://badges.renovateapi.com/packages/npm/testem/3.10.1/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/npm/testem/3.10.1/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/npm/testem/3.10.1/compatibility-slim/3.10.0)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/npm/testem/3.10.1/confidence-slim/3.10.0)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>testem/testem</summary>

### [`v3.10.1`](https://togithub.com/testem/testem/releases/tag/v3.10.1)

[Compare
Source](https://togithub.com/testem/testem/compare/v3.10.0...v3.10.1)

#### What's Changed

- build(deps): bump socket.io from 4.5.3 to 4.5.4 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[testem/testem#1609
- build(deps-dev): bump sinon from 14.0.2 to 15.0.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[testem/testem#1611
- build(deps-dev): bump socket.io-client from 4.5.3 to 4.5.4 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[testem/testem#1610
- build(deps-dev): bump eslint from 8.28.0 to 8.30.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[testem/testem#1616
- build(deps-dev): bump sinon from 15.0.0 to 15.0.1 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[testem/testem#1617
- Scope testem assets with a prefix by
[@&#8203;stepankuzmin](https://togithub.com/stepankuzmin) in
[testem/testem#1615

#### New Contributors

- [@&#8203;stepankuzmin](https://togithub.com/stepankuzmin) made their
first contribution in
[testem/testem#1615

**Full Changelog**:
testem/testem@v3.10.0...v3.10.1

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "every weekday" (UTC), Automerge - At
any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://app.renovatebot.com/dashboard#github/TryGhost/Ghost).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC42Ni4wIiwidXBkYXRlZEluVmVyIjoiMzQuNjYuMCJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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.

None yet

2 participants