codeforboston / cambridge_energy_app

EnerSave is designed to help and encourage Cambridge residents to reduce their electric bills.
https://www.enersaveapp.org/
MIT License
33 stars 25 forks source link

Adding additional test coverage #314

Closed shauncarland-old-account closed 7 years ago

shauncarland-old-account commented 7 years ago

I'm not sure if this is so much of an issue per se, but I've noticed that there are a lot of untested components to this application. For example, there's no tests involving a few of the models (e.g. user tips and uploads). And in other tests, there's definitely room to refactor some of the code to make it DRYER.

Also, I noticed that in the feature spec folder, there are three specs that have to do with users. Wouldn't it make sense to put all of these tests all in the same spec file?

These are just my thoughts. I'm sorta new to contributing to open source software so I'm not 100% sure what the protocol would be for getting consensus on what I am saying...so I think I'll start working on improving the tests and use feedback to change my work accordingly. ^_^

mzagaja commented 7 years ago

Hi @shauncarland, thanks so much for your feedback.

I think your assessment is correct and it is something we have been looking at (see i.e. #286, #150). In regards to components missing tests, some of them were written by contributors without experience in writing tests there were hoping someone else would later come along to add those. More generally we've been lacking in intermediate and senior developers that can help review code and provide constructive suggestions and mentorship to the contributors that are newer to software development.

Generally we create a GitHub issue for anything that is a bug or feature (issues are free!) and then people claim/assign the issue to themselves when they are working on it to signal to the group that they are doing so. I am happy to add you as a collaborator so you can create branches for your features and fixes (please limit each branch to a single issue) and then pull to the main branch. I usually ask someone else review and approve a pull request before it is merged in.

Another way you can help is to review any of the outstanding pull requests if they have not yet been reviewed. I have cleaning those up on my docket for the weekend but another pair of eyes does not hurt. I will likely to pulling many front-end UI/UX changes over the weekend in preparation for an initial launch but in doing so will try to avoid messing too much with back end things, so if you want to concentrate on these back end issues it certainly wouldn't hurt. You are also welcome to go through the open "bugs" in the issue tracker and claim ones that aren't being worked on.

You are also welcome to join our slack channel by going to public.codeforboston.org and requesting an invite. I'm happy to answer further questions there in the #cambridgeenergyapp channel.

Thanks so much for offering to help!

-Matt