-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
CI/CD deployment docs #2389
CI/CD deployment docs #2389
Conversation
Signed-off-by: Mihovil Ilakovac <[email protected]>
Signed-off-by: Mihovil Ilakovac <[email protected]>
Signed-off-by: Mihovil Ilakovac <[email protected]>
**Continuous Deployment (CD)** refers to the automatic deployment of code changes to the production environment. This is commonly know as "push to deploy" and frees developers from having to manually deploy code changes. | ||
|
||
## Running tests in CI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this should be under deployment, since CI is a separate thing from deployment.
Maybe it would be better if we had a part of docs called Testing and then tehre we can have CI part?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we do that when we have all the deployment docs merged? I don't want to spawn a new Testing docs
effort. Let's keep this here for now and then we can rework the Testing docs in the future and move this bit there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I will leave that to you to figure out! I thought maybe creating this new page in docs is clear cut, but if that is too much for now, I get it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created an issue: #2439
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! This one feels a bi tless claer to me, regarding what should be here. Seems every easy to me to go into too much details, or to not give enough info. Since it is more CI/hosting provider specific than Wasp specific. So I think it is ok to keep this as light as we can.
web/docs/deployment/ci-cd.md
Outdated
We'll show you how to run end-to-end tests in CI using the [Github Actions](https://github.com/features/actions) as our CI and the [Playwright](https://playwright.dev/) as our e2e testing framework. | ||
|
||
1. Check our example app and its e2e tests in the [e2e-tests](https://github.com/wasp-lang/e2e-test-example/tree/main/e2e-tests) directory. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm should this point to release branch? Usually in docs we aim to point at release branch, since we know it is in sycn with the released version of the docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm linking here to a separate repository that only has a main
branch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All right I missed that! Hos iw that, don't we have example app with e2e tests in our wasp
repo? How is it that we also have this external one? Should we be linking to it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have e2e tests running in our wasp
repo but it's quite complex IMHO so I wanted to offer a simple example repo with the e2e tests so people can just grab them and use them. Maybe in the future when we work on #2024 we can link to the consolidated app.
web/docs/deployment/ci-cd.md
Outdated
import { expect, test } from "@playwright/test"; | ||
import { generateRandomUser, logUserIn } from "./utils"; | ||
|
||
const user = generateRandomUser(); | ||
|
||
test.describe("basic user flow test", () => { | ||
test("log in and add task", async ({ page }) => { | ||
await logUserIn({ page, user }); | ||
await expect(page).toHaveURL("/"); | ||
await expect(page.locator("body")).toContainText("No tasks yet."); | ||
|
||
// Add a task | ||
await page.fill('input[name="description"]', "First task"); | ||
await page.click('input:has-text("Create task")'); | ||
await expect(page.locator("body")).toContainText("First task"); | ||
}); | ||
}); | ||
``` | ||
</details> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can probably drop this? It is just a small snippet, and it risks becoming out of date, we have to maintain it, real code is anyway in that example app you linked to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to include this code example here to motivate people to write these tests. The example app is nice, but this is available while going through the docs and maybe better understand what e2e tests mean without leaving the docs.
I get your concern about the code being outdated - but the example app code will get outdated over time as well :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My feeling is that maintanability trumps the other arguments here. We will keep the example app up to date though! At least we should, I would expect that.
web/docs/deployment/ci-cd.md
Outdated
- for our server app | ||
- for our client app | ||
4. Notify your deployment provider to deploy the new image. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can depend on their hostirng provider, right? If I am correct some of them have their own registries so this looks more like just a single command, or maybe you just give them a branch with Dockerfile, stuff like that?
Also client app doesn't have to be in Dockerfile.
Also might be worth mentiong here that we provide Dockerfile for the server.
And now that I am writnig all this, sounds to me like this is quite complex and we should instead point them to the part of deployment docs that describes it in details? Maybe just be very simple here, try not to assume much, link to the part with dpeloying Wasp app, and say that they will want to use whatever method their hosting providers reocmmends for CD. And an example deployment of ours, that is cool.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've rewritten this line as For some providers: notify them to deploy the new app version.
Also client app doesn't have to be in Dockerfile.
I've mentioned an alternative way to deploy the client 👍
sounds to me like this is quite complex
Yep, it's quite complex, that's why I wanted to explain the concept from a higher level a bit. We our self-hosted deployment docs that talk a lot about using CD.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved, but check a couple of new comments.
Talks about how users can test their apps in the CI and about packaging their apps in Docker images for CD.
Left to do: