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

Apps for each of the adapters #639

Closed
Rich-Harris opened this issue Mar 24, 2021 · 12 comments
Closed

Apps for each of the adapters #639

Rich-Harris opened this issue Mar 24, 2021 · 12 comments
Labels
adapters - general Support for functionality general to all adapters

Comments

@Rich-Harris
Copy link
Member

The adapters don't have tests, and it's not totally clear what value they'd provide if they did, but I think we should have hello world apps for each of the officially supported adapters running on the respective platforms.

Should they live in this repo? In examples or a new directory? Or in the package directories themselves? Or should they have their own individual repos?

Most importantly, who feels like setting it up? 😀

@antony
Copy link
Member

antony commented Mar 24, 2021

I think under examples is fine - examples/begin-adapter perhaps.

We could deploy each via API and ensure that the relevant CLI tool returns ok and not an error. We could also possibly ping the deployed endpoint looking for a 200 or similar. Optionally a very quick playwright test run against the deployed app. It'd be a pretty reasonable test.

@benmccann
Copy link
Member

I think most of the adapters should be in separate repos. Keeping the most used ones here is fine as long as we're going to be maintaining them. I don't know if we want to be responsible for an influx of community adapters though

For the examples, should we have separate standalone apps or can we just repurpose some of the existing ones. E.g. deploy the HN to Netlify and the Realworld example to Vercel

@antony
Copy link
Member

antony commented Mar 24, 2021

I'm not suggesting we should adopt all community adapters at all - I think we're talking about the basic set of us-maintained ones here. It makes sense for the example apps to live in the monorepo, so that they aren't a maintenance burden.

I think it should be the same app across all adapters - that way we can make checks for consistency, and spot misbehaving adapters early.

@Rich-Harris
Copy link
Member Author

Yeah, I was thinking a super basic hello world that has page with a dynamic parameter, an endpoint with a dynamic parameter, a handful of static files, something prerenderable, maybe a setup function to make sure things work in an env-agnostic way, etc. svelte-kit-demo is probably pretty close already. Maybe we could even have a single codebase, and we could deploy it by swapping out the adapter at build time according to whatever env var is currently applicable

@benmccann
Copy link
Member

Maybe we could use demo or sandbox app

The other part of this is detecting when it breaks. If we have an app for each adapter we need someone to periodically visit those apps in order to detect issues. We could use a downtime monitoring service like Pingdom

@benmccann
Copy link
Member

Although I guess if the deploy just fails there wouldn't be downtime - we'd get an old version instead. Maybe we need to do a GitHub Actions redeploy on each commit and see when it fails

@Rich-Harris
Copy link
Member Author

Depending on how quotas are structured, we could blow through free tiers pretty quickly if we were deploying on every commit. It might need to be a more manual thing, or something that happens whenever Kit or the relevant adapter gets a release

@Rich-Harris
Copy link
Member Author

Something that just popped up: it's not enough to build the apps in CI, we need to check that they're actually working as intended — at the very least that they're responding to requests (and that it's the new deployment that is serving responses, not a previous one because the new one failed)

@antony
Copy link
Member

antony commented Mar 31, 2021

The clis for most of these services will check that the root url returns a 200 before confirming the deployment, so that's the most basic check. We simply inspect the exit code (or in some cases the github action will fail)

From there I think we can have a series of tests such as /some/endpoint/204 returning a 204, various checks on headers, etc, as well as all the proposed tests which were talked about elsewhere.

As for quotas, in a lot of cases the actual deployment quotas are designed for deploying PRs and such for demo purposes, so I don't think there'll be a huge issue. I might also suggest that should we run into quota issues, I think the relevant providers are interested enough in getting first class support for Svelte Kit on their platform that they'd be willing to increase them as required.

And in terms of setting this up - the moment I have a spare hour I'll work on getting some of this stuff into github actions.

samccone added a commit to samccone/kit that referenced this issue Mar 31, 2021
* verify the server starts
* verify it can serve a 404

---

Additional changes:

- create a server_base that creates the server but does not start it
- keep the existing server.js file which starts the server (non-breaking
interface)
- update the package type to module to enable esm imports

BUG sveltejs#639
samccone added a commit to samccone/kit that referenced this issue Mar 31, 2021
* verify the server starts
* verify it can serve a 404

---

Additional changes:

- create a server_base that creates the server but does not start it
- keep the existing server.js file which starts the server (non-breaking
interface)
- update the package type to module to enable esm imports

BUG sveltejs#639
samccone added a commit to samccone/kit that referenced this issue Mar 31, 2021
* verify the server starts
* verify it can serve a 404

---

Additional changes:

- create a server_base that creates the server but does not start it
- keep the existing server.js file which starts the server (non-breaking
interface)
- update the package type to module to enable esm imports

BUG sveltejs#639
samccone added a commit to samccone/kit that referenced this issue Mar 31, 2021
* verify the server starts
* verify it can serve a 404

---

Additional changes:

- change server.js to be a function that creates the server but does not start it.
- update build rules to build from a new index.js file that builds and
starts the app
- update the package type to module to enable esm imports
- expose a way to call into server with a render function
- add a guard clause around no static asset folder existing.

BUG sveltejs#639
samccone added a commit to samccone/kit that referenced this issue Mar 31, 2021
* verify the server starts
* verify it can serve a 404

---

Additional changes:

- change server.js to be a function that creates the server but does not start it.
- update build rules to build from a new index.js file that builds and
starts the app
- update the package type to module to enable esm imports
- expose a way to call into server with a render function
- add a guard clause around no static asset folder existing.

BUG sveltejs#639
samccone added a commit to samccone/kit that referenced this issue Mar 31, 2021
* verify the server starts
* verify it can serve a 404

---

Additional changes:

- change server.js to be a function that creates the server but does not start it.
- update build rules to build from a new index.js file that builds and
starts the app
- update the package type to module to enable esm imports
- expose a way to call into server with a render function
- add a guard clause around no static asset folder existing.
- change imports in adapter-node to work with import and not require

BUG sveltejs#639
samccone added a commit to samccone/kit that referenced this issue Mar 31, 2021
* verify the server starts
* verify it can serve a 404

---

Additional changes:

- change server.js to be a function that creates the server but does not start it.
- update build rules to build from a new index.js file that builds and
starts the app
- update the package type to module to enable esm imports
- expose a way to call into server with a render function
- add a guard clause around no static asset folder existing.
- change imports in adapter-node to work with import and not require

BUG sveltejs#639
samccone added a commit to samccone/kit that referenced this issue Mar 31, 2021
* verify the server starts
* verify it can serve a 404

---

Additional changes:

- change server.js to be a function that creates the server but does not start it.
- update build rules to build from a new index.js file that builds and
starts the app
- update the package type to module to enable esm imports
- expose a way to call into server with a render function
- add a guard clause around no static asset folder existing.
- change imports in adapter-node to work with import and not require

BUG sveltejs#639
samccone added a commit to samccone/kit that referenced this issue Mar 31, 2021
* verify the server starts
* verify it can serve a 404

---

Additional changes:

- change server.js to be a function that creates the server but does not start it.
- update build rules to build from a new index.js file that builds and
starts the app
- update the package type to module to enable esm imports
- expose a way to call into server with a render function
- add a guard clause around no static asset folder existing.
- change imports in adapter-node to work with import and not require

BUG sveltejs#639
samccone added a commit to samccone/kit that referenced this issue Mar 31, 2021
* verify the server starts
* verify it can serve a 404

---

Additional changes:

- change server.js to be a function that creates the server but does not start it.
- update build rules to build from a new index.js file that builds and
starts the app
- update the package type to module to enable esm imports
- expose a way to call into server with a render function
- add a guard clause around no static asset folder existing.
- change imports in adapter-node to work with import and not require

BUG sveltejs#639
Rich-Harris pushed a commit that referenced this issue Mar 31, 2021
* verify the server starts
* verify it can serve a 404

---

Additional changes:

- change server.js to be a function that creates the server but does not start it.
- update build rules to build from a new index.js file that builds and
starts the app
- update the package type to module to enable esm imports
- expose a way to call into server with a render function
- add a guard clause around no static asset folder existing.
- change imports in adapter-node to work with import and not require

BUG #639
@Rich-Harris Rich-Harris mentioned this issue Apr 14, 2021
13 tasks
@jthegedus
Copy link
Contributor

jthegedus commented May 10, 2021

Working on these types of tests for the Firebase adapter I had a few ideas.

The tests I would like to write in my GitHub Action is essentially:

  • pnpm init svelte@next tmp/my-test --<pre-selected options object>
  • npx svelte-add <adapter> or pnpm install ../<relative dir to adapter>
  • write a test firebase.json to the this tmp/my-test project
  • firebase deploy
  • playwright to hit each URL in the manifest and check for 200 (Firebase CLI does not check app executes without error, but rather it deployed without error)
  • loop this for some number of permutations of the adapter config (I want to target at least 4 for the Firebase one)

The idea here being that I would avoid git tracking and having to update my copy of the template app for each test of this type.

Currently this workflow will not work because the first step cannot be used in CI, at least as far as I can tell.

What are the current thoughts and developments on this front? I haven't seen much discussion in Discord the brief moments I check, but would like to be an ear in the conversation.

@babichjacob
Copy link
Member

* `pnpm init svelte@next tmp/my-test --<pre-selected options object>`

Currently this workflow will not work because the first step cannot be used in CI, at least as far as I can tell.

See #1231 for an attempt to make the init process non-interactive that was rejected and why.

@Rich-Harris
Copy link
Member Author

Going to close this as we're currently deploying demo apps on each commit to platforms supported by the official adapters, and it'd be relatively straightforward for community adapters to do likewise via https://github.com/sveltejs/kit-template-default

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
adapters - general Support for functionality general to all adapters
Projects
None yet
Development

No branches or pull requests

5 participants