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

Ensure that tests terminate #35

Closed
thomashoneyman opened this issue Jan 12, 2021 · 6 comments
Closed

Ensure that tests terminate #35

thomashoneyman opened this issue Jan 12, 2021 · 6 comments
Labels
good first issue First-time contributors who are looking to help should work on these issues. type: housekeeping Repo-related things (e.g. fixing CI) that need to be done.

Comments

@thomashoneyman
Copy link
Contributor

Currently, CI fails (as described in #33) because the the tests leave servers running until they are manually torn down. This results in timeout failures as in this failed run. We should fix the tests so that they terminate.

@thomashoneyman thomashoneyman added good first issue First-time contributors who are looking to help should work on these issues. help wanted labels Jan 12, 2021
@VFabricio
Copy link
Contributor

As I mentioned in #33, it is awkward to torn down the servers given how the tests are currently written. I could take a shot at fixing them, but I'd like to understand a bit better why they are structured like that, because there may be some context I'm not aware of. For the most part, they don't really run any assertions and simply print results to be inspected manually. When something is actually asserted, it uses unsafeCrashWith to simply kill the process. What is the rationale for doing things like that? Would it be possible to use a test runner like purescript-spec instead?

@hdgarrood
Copy link
Member

I think the idea with these tests might have been that you run npm test to start server processes and then test them manually (e.g. by loading pages in your browser).

@VFabricio
Copy link
Contributor

Sounds like a sensible thing to do, but then we would need separate tests suitable for CI, right?

@hdgarrood
Copy link
Member

Yes; I wouldn't mind replacing them with tests which are suitable for CI, actually.

This was referenced Sep 22, 2021
@JordanMartinez JordanMartinez added type: housekeeping Repo-related things (e.g. fixing CI) that need to be done. and removed help wanted labels Nov 30, 2021
@jamesdbrock
Copy link
Member

I'm working on this in #45

@jamesdbrock jamesdbrock mentioned this issue Nov 21, 2022
4 tasks
@jamesdbrock jamesdbrock linked a pull request Nov 21, 2022 that will close this issue
4 tasks
@jamesdbrock jamesdbrock mentioned this issue Jun 19, 2023
4 tasks
@JordanMartinez
Copy link
Contributor

Closed by #48

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue First-time contributors who are looking to help should work on these issues. type: housekeeping Repo-related things (e.g. fixing CI) that need to be done.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants