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

Draft: Threads views tests #92

Closed
wants to merge 1 commit into from

Conversation

amandasavluchinske
Copy link
Contributor

Folks, I created this draft to get your opinion on something. For test data, we have a few options:

  1. Use a lib like factory or baker - I'm not a huge fan of this option as this lib's models are fairly simple, so I don't think we gain a lot by adding this extra dependency.
  2. Create objects within the test file - I think this one is simple, but can become a little messy.
  3. Create a helper service within the test file - Could work, but I also find it a little messy.
  4. Create a helper service in helpers.py (within the tests folder) - That's what I went with, but I also saw there's an utils.py file. I figured the purposes were a little different, but I can add the helper to utils if it makes sense.

Anyway, let me know what you think. This is not a blocker, I'm working on other tests in the meantime, but I'd like to get your opinion before opening the definitive PR.

@amandasavluchinske amandasavluchinske changed the base branch from main to td/add-tests-to-views-assistant June 17, 2024 17:11
@fjsj
Copy link
Member

fjsj commented Jun 17, 2024

Use a lib like factory or baker - I'm not a huge fan of this option as this lib's models are fairly simple, so I don't think we gain a lot by adding this extra dependency.

It's not a problem because that will be a test-only dependency. I think it's fine to use model bakery.

Base automatically changed from td/add-tests-to-views-assistant to main June 17, 2024 17:35
@amandasavluchinske amandasavluchinske deleted the td/add-tests-to-views-threads branch June 17, 2024 17:38
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