-
Notifications
You must be signed in to change notification settings - Fork 109
Testing_Conventions
DMPRoadmap's test environment and standards are still evolving. The project started out with close to zero tests. The tests in the code today have been created over time by different developers and with changing approaches/methodologies. It is still not in an ideal situation and continues to grow along with the project.
- Tests MUST pass before a PR can be accepted and merged
- All models should have unit tests defined
- You should test any functions that have been defined for both positive and negative outcomes
- You should test model attribute validations
- You should test any callbacks: after_save, before_validation, etc.)
- You should test any scopes to ensure that they return the correct data
- All controllers should have functional controller tests defined
- Controller tests should only verify: status codes, that the correct render or redirect happened and, in the case of create/update/destroy endpoints, that the underlying object was properly modified.
- You should include tests for failures as well: Unauthorized, invalid or missing params, etc.
- All concerns, services and helpers should have tests defined
- These objects are more complex and often require that you instantiate other classes (e.g. You would need to instantiate the Paginable::PlansController in order to test the Paginable concern). You should focus your tests though as specifically as possible on the underlying concern/service/helper.
- You should test both positive and negative scenarios
Our current framework consists of a few large issues. The first has to do with data and the second has to do with the engine. You can get a better understanding of these discrepancies by reviewing the test_helper.rb. In this file you will notice:
- Engine:
- We are using MiniTest and it is not inherently an issue. The current trend in Rails applications is to use RSPec and FactoryBot for testing though and it may prove to be beneficial to move in that direction to make tests easier to follow and create by external partners. We originally went with MiniTest due to its lightweight footprint.
- Data:
- The
db/seeds.rb
file is loaded.- Reason: This was initially done to prevent the need for having to maintain a separate
seeds.rb
file and a set of test records. - Issues: The data is loaded to the test database and persists beyond the duration of the tests. Its separate from the individual tests so its not always clear what data is available. Additionally, changes to this data due to tests persist as well
- Reason: This was initially done to prevent the need for having to maintain a separate
- We commented out the use of fixtures.
- Reason: Fixtures are inherently fragile due to the fact that they are loaded outside of ActiveRecord. This means that you could insert a user for example who has no email address. You might not discover this however until a
registration_controller.rb
test is run and you attempt to edit the user record which sends you a 302 status code. You would assume the controller was at fault when in fact it was theuser.valid?
method that was failing. - Issues: Fixtures are the basic data structures used by MiniTest
- Reason: Fixtures are inherently fragile due to the fact that they are loaded outside of ActiveRecord. This means that you could insert a user for example who has no email address. You might not discover this however until a
- A mixture of factory like functions for instantiating new objects
- Reason: Test data should be instantiated when its needed! Each test should create only the records it needs for the given test.
- Issues: These are homegrown functions and not using a standard gem like FactoryBot to help us manage mocks and data management. There is also a mix of different types of these factory methods in use. We encourage the use of the ones prefixed with
init_
(e.g.def init_researcher(org, **props)
) which allow you to passing in a hash of attribute values that you can use to overwrite the defaults for any of the model's attributes.
- The
Please also note that our current test coverage is not exhaustive. There are still a lot of functions and blocks out there that have no corresponding tests. If you happen to update one of those untested sections of code, please consider building out tests for it and submitting it along with your PR.
Summary of our lack of UI tests coming soon ...
Explanation of npm test
and our very spotty coverage coming soon ...
The ideal scenario coming soon ...
If you have the time and inclination, then we encourage you to move us towards the ideal future mentioned above. We understand though that is not always realistic. So, what should you do in the short term?
- Make use of the existing factory helpers that are prefixed with
init_
(e.g.def init_researcher(org, **props)
) which allow you to passing in a hash of attribute values that you can use to overwrite the defaults for any of the model's attributes. - Use the
def setup
method in your test file to create any objects that will be used across all of your tests - Test both positive and negative outcomes
- If you've updated a section of code and notice that it has no tests. Consider adding the test for us!
- Home
- About
- Development roadmap
- Releases
- Themes
- Google Analytics
- Get involved
- Translations
- Developer guide