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

Discuss test assertions #60

Open
rmanaem opened this issue Jan 21, 2023 · 2 comments
Open

Discuss test assertions #60

rmanaem opened this issue Jan 21, 2023 · 2 comments
Labels
flag:discuss Flag issue that needs to be discussed before it can be implemented. _flag:stale [BOT ONLY] Flag issue that hasn't been updated in a while and needs to be triaged again maint:coverage Test coverage improvements that were not included in feature prioritization type:maintenance Upkeeping efforts & catch-up corrective improvements that are not Features nor Bugs

Comments

@rmanaem
Copy link
Collaborator

rmanaem commented Jan 21, 2023

Asserting over what is returned from the mocked get function in test_query has been brought up multiple times during conversations and PRs, most recently here.
I'd like us to think about the questions below:

  • Is what we have unnecessary? since it's simply being fed into the test and asserted on
    If not
  • Is what we currently have (simply check if the returned list is empty or not) enough?
    If not
  • Do we want to check each of the properties in the AggDatasetResponse?

Or perhaps we need to re-think and re-work the implementation.

Other thoughts related to testing:

Because we are mocking our get method in our tests (good), and the query generator is called only inside the get method - technically inside the httpx.post call (neutral), the query generator is never tested (bad).

One answer is to make separate unit tests for the query generator.

What do you think?

@rmanaem rmanaem added type:maintenance Upkeeping efforts & catch-up corrective improvements that are not Features nor Bugs flag:discuss Flag issue that needs to be discussed before it can be implemented. maint:coverage Test coverage improvements that were not included in feature prioritization labels Jan 21, 2023
@surchs
Copy link
Contributor

surchs commented May 8, 2023

Related to #48

@github-actions
Copy link

We want to keep our issues up to date and active. This issue hasn't seen any activity in the last 30 days.
We have applied the stale-issue label to indicate that this issue should be reviewed again and then either prioritized or closed.

@github-actions github-actions bot added the _flag:stale [BOT ONLY] Flag issue that hasn't been updated in a while and needs to be triaged again label Aug 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flag:discuss Flag issue that needs to be discussed before it can be implemented. _flag:stale [BOT ONLY] Flag issue that hasn't been updated in a while and needs to be triaged again maint:coverage Test coverage improvements that were not included in feature prioritization type:maintenance Upkeeping efforts & catch-up corrective improvements that are not Features nor Bugs
Projects
Status: No status
Development

No branches or pull requests

2 participants