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

Add unit test #5

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Add unit test #5

wants to merge 9 commits into from

Conversation

scottnuma
Copy link
Collaborator

  • Sort into a couple folders: mentor_matching, data, docs
  • Add unit testing structure

@scottnuma scottnuma requested a review from j-m-h March 21, 2020 23:16
Copy link
Collaborator

@j-m-h j-m-h left a comment

Choose a reason for hiding this comment

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

So it looks like right now assign.py is pointing to the wrong location for the mentor and team data--it assumes that assign.py is in the root directory, but it's actually one level down. (This is a fairly easy fix)

A bigger potential problem is that if (more like when) the input format changes from year to year, we're also going to have to change the unit test to fit that format. idk if that's that big a problem, but my goal here was kinda to make changes as easy as possible so that this can be used from year to year even if the people who originally worked on it are no longer around.

Perhaps as an alternative to unit tests (although I'm sure this isn't as good coding practice), we could just put something in the readme saying to make sure to run the code locally and ensure that it works before pushing (that's honestly how I found the issues with the enums and the file location).

@scottnuma
Copy link
Collaborator Author

scottnuma commented Mar 22, 2020

Hmm - you make a good point on the need to change around in response to the form. I think rather than per function unit, integration tests would be better:

  • test to run the entire program on sample data
  • test to parse mentor data
  • test to parse uhh school data

Keeping these should ensure stuff stays in a non-broken state, while leaving all the internal workings to be abstracted away.

@scottnuma
Copy link
Collaborator Author

Moved assign.py --> top level main.py
Applied a linter

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