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

Testing sprout 🌱 #84

Merged
merged 24 commits into from
Oct 21, 2023
Merged

Testing sprout 🌱 #84

merged 24 commits into from
Oct 21, 2023

Conversation

Vectornaut
Copy link
Collaborator

@Vectornaut Vectornaut commented Oct 19, 2023

Summary

This pull request develops @liammulh's prototype test (TrivialTest) into a small suite of useful tests.

Notable features

  • Basic documentation on writing and running tests (doc/tests.md).
  • A convention for writing families of tests by subclassing an abstract test (in this case, AbstractEndpointTest).

Pitfalls

  • The test process fails gracefully when POSTGRES_DISPOSABLE_DB is unset or empty, but the code that makes this happen is very convoluted. It ought to be simplified at some point.
  • I've changed the definition of ProdConfig to accommodate other changes, but I don't know how to make sure production mode still works.

To do

See # TO DO comments.

  • Expand AbstractEndpointTest to test the background work triggered by each endpoint request.

Copy link
Member

@katestange katestange left a comment

Choose a reason for hiding this comment

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

I was able to get it up and running and run the tests! I made some comments on the docs.

doc/tests.md Show resolved Hide resolved
doc/tests.md Show resolved Hide resolved
doc/tests.md Outdated Show resolved Hide resolved
flaskr/nscope/test/test_get_oeis_values.py Outdated Show resolved Hide resolved
doc/install-postgres.md Show resolved Hide resolved
doc/tests.md Show resolved Hide resolved
@gwhitney
Copy link
Collaborator

OK, I made a couple of small comments/suggestions, but overall this looks really good! So once Kate's resolved her items and Aaron's had a chance to respond to the things I raised, this should be ready to merge. Thanks, Aaron!

Aaron Fenyes added 2 commits October 20, 2023 20:26
Continuous integration will be in a different pull request.
@gwhitney
Copy link
Collaborator

OK, as far as I can see, everything is set here for merging. But as Kate has to sign off as the original reviewer, I can't merge on the spot. Kate, when you have a chance, make sure you feel all is well, sign off, and merge. Thanks!

@katestange katestange merged commit d1df281 into numberscope:main Oct 21, 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.

4 participants