Testing #198
Testing #198
Conversation
There was a problem hiding this comment.
The tests are almost done. However, there are some methods/lines that need more testing, some tests that should be separated from their source code (because circular tests are not very helpful), and some tests that are overly specific (which can lead to needing to frequently change tests as source code evolves, which is an inefficient use of effort). In addition, there is some code that can be simplified.
-
Testing is one of the most helpful strategies for implementing efficiently (to leave as much time as possible for science). But, high test coverage is needed to fully take advantage of this. Please add tests for the methods below.
formatTitlemethods inMetadataSection.jsfilesgetMetadataUrlinRna/MetadataSection.js
-
To use the full utility of testing, it is important to have not just high method coverage, but also have high line and branch coverage. For one thing, this forces you to think about what cases require each block of code, and whether these cases will ever be encountered. For example, I found this particularly helpful for understanding which React lifecycle methods need to be implemented. This helped me find and delete code that was never executed and served no purpose. Please add additional test cases to cover these lines.
- 189-195 of
Metabolite/MetadataSection.js - 131-151 of
Reaction/MetadataSection.js - 28-30 of
Reaction/RateConstantsDataTable.js - 134 of
Rna/MetadataSection.js
- 189-195 of
-
For tests to be maximally useful, they need to be independent from the source code. Using a source method to test itself has limited utility because this allows for the possibility of checking an erroneous method with the same erroneous method, which can prevent you from finding and fixing errors, enabling errors to persist. When so many errors exist that they are scattered throughout code, debugging becomes very onerous; the debugging scope becomes equivalent to the entire codebase. Typically, one effective strategy is to test edge cases in which the result simplifies and the result can be computed by a simpler method that is independent from the source code. If there are enough such tests, debugging becomes easy because the scope get limited to a very small number of lines of code. Please make these tests independent from the source code (i.e. do not copy source code into tests).
- Lines 64-71 of
Metabolite.test.js - Lines 56-61 of
Protein.test.js - Lines 46-51 of
Reaction.test.js
- Lines 64-71 of
-
Testing is most effective when the tests focus on the meaning of computations and ignore aspects of outputs that have no meaning and, therefore, could change if a method is revised. For example, the order of the attributes in Line 121 of
BiochemicalEntityDetails/Metabolite.test.jsis not important. Consequently, Datanator would behave the same if the order were changed. For that reason, developers are free to change the order. To minimize the need to update tests as code is refactored, it is useful to make tests less sensitive to implementation choices. In this case, it would be sufficient to test (a) the value of thehrefattribute and (b) the inner text (UDP). -
I think it is important for scientists to understand the methods that they're using. This is important for minimizing confusion, freeing up attention for bigger problems. If you're not sure whether you understand something, try it out or ask questions. Please simplify these issues:
- It is not necessary to wrap the input to
shallowindivas at Line 99 ofBiochemicalEntityDetails/Metabolite.test.js. Please remove this. asyncis not necessary for any of theBiochemicalEntityDetailstests.asyncis only needed to test methods that involve asynchronous REST API calls. Please remove this.
- It is not necessary to wrap the input to
Pull Request Test Coverage Report for Build 915
💛 - Coveralls |
|
Thanks for the feedback. I have made all the additions with one excpetion: |
No description provided.