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

new unit tests with new library versions + fixes #792

Merged
merged 14 commits into from
Aug 25, 2020
Merged

Conversation

mattyweb
Copy link
Collaborator

First set of work on #789

Before doing more refactoring wanted to have some better test coverage of the API services methods which is where the heart of the logic resides.

  • Added new tests for item, clusters, heatmap, and visualizations
  • Got a few async tests running with asserts on data returned
  • Had to upgrade lots of stuff to get it to work including sanic and pytest
  • Refactored Pipfile to have dev packages and reference only directly imported packages
  • Fixed bugs introduced by new libraries
  • Added new functions for getting all NCs and Request Types

Key bugs that library upgrades introduced were:

  1. a CORS problem where OPTIONS now needs to be explicitly included
  2. an issue in the heat route where NumPy array needs conversion to list before serialization

Currently, the new functions are included as services functions. They are needed to help develop test but I put them there since I can see a future where we want to expose these as API endpoints.

I have a 'do not merge' label on this for now as the scope of the changes is beyond what I had initially intended. Unfortunately and especially since async support is pretty new in Python, I think we need to keep libraries relatively up to date. I don't know what the specific problem was but I couldn't get my tests to work until I updated everything.

I'd also like to discuss running Python 3.8.

And lastly, would like to know if there are other critical pieces of functionality that need tests before this can merge.

  • Up to date with dev branch
  • Branch name follows guidelines
  • All PR Status checks are successful
  • Peer reviewed and approved

@mattyweb mattyweb added the Do Not Merge Do not merge this PR, it is likely blocked by something else label Aug 25, 2020
@mattyweb
Copy link
Collaborator Author

Well, now flake8 is working and tests are running...

But asserts are all getting errors because of data differences. Somehow the numbers don't add up between my local and the dev site.

I think it's possible that the data is the same, but the clusters are different. I'm seeing both total 690K from Jan 1 to June 30 but the sites have different cluster grouping. Not sure why this could be the case.

Might need to change the asserts if we can't figure out a better plan.

@mattyweb
Copy link
Collaborator Author

thanks for the feedback @jmensch1 ! Any other pressing tests for me to add?

@jmensch1
Copy link
Contributor

I don't have any tests to add for now. Great start, we can get this merged and add more tests later.

@mattyweb
Copy link
Collaborator Author

Current db seeding from file is loading 100K records and taking 33s to load to the DB on CI. Might want a larger seed file.

Everything is passing except for 2 postman tests (though i had to disable 1 pytest where the data is not matching).

@mattyweb mattyweb removed the Do Not Merge Do not merge this PR, it is likely blocked by something else label Aug 25, 2020
@mattyweb mattyweb merged commit c2ac3d6 into dev Aug 25, 2020
@mattyweb mattyweb deleted the 789-begin-test-refactor branch August 25, 2020 21:16
@jmensch1 jmensch1 linked an issue Aug 25, 2020 that may be closed by this pull request
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.

[FEAT] Create integration tests and/or unit tests with data fixtures
2 participants