Facebook Open Academy Blog

by Jussi Mertanen


Project maintained by Sonopa Hosted on GitHub Pages — Theme by mattgraham

Quality

Slowly but somewhat surely is how I would describe our progress this week. We have gotten some parts of the locals problem working to some degree, but there is still a lot of work to do. The changes we have made so far affect many parts of the code and broke about 300 tests, so we’ll need to fix all that before we even see if we are on the right tracks. This week we also noticed a couple of failing tests on our feature. It turned out that the option for our feature was turned on when running rake tasks, which is something that we don’t want. Luckily some tests that tested rake tasks in production mode caught it and we were able to fix it. This weeks topic is actually related to this, today I will write about the quality and testing of our code and the rest of the Rails code.

Quality

As with many open source projects, the quality of Ruby on Rails source code varies quite a lot. Since a lot of people contribute, it’s quite unlikely that everyones code is written in the best way possible. And because the amount of pull requests is quite large, not everything can be reviewed with care. Rails has a guide for contributors, and that guide contains some coding conventions, which help keep the code style similar across all parts of Rails. Conventions change over time though, so there is variance in style as well. Even so, I think for the most part the code quality of Ruby on Rails is good. During this course, I’ve never had a situation where I’ve had problems understanding the code due to bad quality. Some parts are of course quite complex and could be made easier to understand with refactoring, but those are rare and probably not worth it. When refactoring, there is always the risk of braking something, and pull requests that are only small refactors are usually discouraged. I’m sure a lot of people would be eager to do some refactoring, but the value of it, I think, is limited, so it’s probably a good idea to only refactor what is necessary. Something I think could or should be improved in the code is commenting. Comments, especially in the more complex parts, would make understanding the code a lot quicker. Rails code could use more of that.

We’ve tried to keep a high quality in our code as well, and I think we’ve succeeded in that. The main part of our code to me looks very understandable, the methods are well named, clear and short. I don’t see how we could improve on that. I guess we could do some more commenting in our code as well, since that is the only problem I have with the other parts of the Rails code. But commenting is not very fun, so I understand why there is not as much of it as there should.

Testing

The tests in Rails are still quite unfamiliar to me, since I haven’t been the one making them for our feature (but I should in the future). Rails has a lot of tests. When you code something for Rails, tests for your code are usually required. While the mount of tests is good, I don’t think the quality of the tests is assured. I haven’t seen any comments regarding the tests in pull requests, so I guess people aren’t focusing on them that much.

The tests in Rails are Minitest inheriting unit tests. As far as I know, unit tests are the only kind of tests Rails has. Because Rails doesn’t want to depend on outside libraries, using things like mocha (A Ruby library for mocking and stubbing) in tests is not recommended (we found a FIXME that said to stop using mocha). That makes writing tests a bit harder though. The testing framework Rails uses is very basic. In the end we went with using mocha anyway, as it made things a lot easier and Matthew said we could use it. This has paid off since we now have dozens of tests, so we should be good on that part.