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

chore: add e2e tests for GET /items/:id, GET /account, GET /dashboard, GET /dashboard/stats and GET /dashboard/users #557

Merged
merged 7 commits into from
Sep 10, 2023

Conversation

revgum
Copy link
Contributor

@revgum revgum commented Sep 10, 2023

Adding more coverage on behalf of #524

@revgum revgum changed the title chore: add e2e tests for GET /items/[id] chore: add e2e tests for GET /items/[id] and GET /account Sep 10, 2023
});
});

Deno.test("[e2e] GET /account", async (test) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added missing tests for /account but then moved all of the other /account/* tests down to be alongside this one.

@revgum
Copy link
Contributor Author

revgum commented Sep 10, 2023

I expect codecov to capture the diff when this is merged.. Locally, using genhtml from Lcov shows the updated coverage.
Screenshot 2023-09-10 at 10 51 10 AM
Screenshot 2023-09-10 at 12 07 08 PM

@revgum
Copy link
Contributor Author

revgum commented Sep 10, 2023

Ah I see, codecov ignores all *.tsx, so even the root routes won't be captured.

@iuioiua what do you think about updating codecov.yml to just ignore components and islands instead of all .tsx files?

headers: { cookie: "site-session=" + user.sessionId },
}),
);
assertStringIncludes(await resp.text(), "<!--frsh-chart_default");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The page itself renders an outlet for the island, and this assert is looking for the first piece of that outlet.

headers: { cookie: "site-session=" + user.sessionId },
}),
);
assertStringIncludes(await resp.text(), "<!--frsh-userstable_default");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The page itself renders an outlet for the island, and this assert is looking for the first piece of that outlet.

@revgum revgum changed the title chore: add e2e tests for GET /items/[id] and GET /account chore: add e2e tests for GET /items/[id],GET /account, GET /dashboard, GET /dashboard/stats and GET /dashboard/users Sep 10, 2023
@revgum revgum changed the title chore: add e2e tests for GET /items/[id],GET /account, GET /dashboard, GET /dashboard/stats and GET /dashboard/users chore: add e2e tests for GET /items/[id], GET /account, GET /dashboard, GET /dashboard/stats and GET /dashboard/users Sep 10, 2023
@revgum
Copy link
Contributor Author

revgum commented Sep 10, 2023

Alrighty.. definitely feel free to take this PR and run with it in terms of syntax adjustments or changes here and there. I've got to get back into my regular obligations. It's been a bunch of fun! I'll try to circle back around to this repo and add contributions as I find more time.

@iuioiua iuioiua changed the title chore: add e2e tests for GET /items/[id], GET /account, GET /dashboard, GET /dashboard/stats and GET /dashboard/users chore: add e2e tests for GET /items/:id, GET /account, GET /dashboard, GET /dashboard/stats and GET /dashboard/users Sep 10, 2023
iuioiua added a commit that referenced this pull request Sep 10, 2023
Copy link
Contributor

@iuioiua iuioiua left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add these in, but these will be analysed in Codecov after v1. I'd like to focus on REST API endpoints as they involve more logic. Either way, this is great! Thank you, Josh.

@iuioiua iuioiua merged commit fccb7b3 into denoland:main Sep 10, 2023
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.

2 participants