KarrLab / datanator_frontend

Graphical web-based interface for the Datanator toolkit for discovering data for modeling cells
https://datanator.info
MIT License
1 stars 1 forks source link

What is the best way to test functions that pass data from grandchildren to grandparents? #186

Closed yosefdroth closed 4 years ago

yosefdroth commented 4 years ago

What would be the best way to test a function like formatMetadataInner in src/scenes/BiochemicalEntityDetails/Protein/MetadataSecition.js (for example)?

I have factored out most the of the actual formatting into a static method FormatMetadata, and this is easy to test by simply passing a JSON to it. But if we want complete coverage, then I will need to test formatMetadataInner too.

The way that this function works is that it receives data from its child BaseMetadataSection, and then passes up the metadata to its parent component, Protein. Furthermore, the BaseMetadataSection requires a router and history in order to format the URL and retrieve the metadata from the API.

What is the best way to test this in Jest? At the moment, I am instantiating a Protein component and router history to pass to the MetadataSection component. I am trying something like this (however, its not currently working):

const protein = new Protein()
let protein_history = history
protein_history["location"] = { pathname: "/protein/K00900/Escherichia coli", search: '', hash: '', state: undefined }
const metadata = renderer.create(<Router history={protein_history}><MetadataSection set-scene-metadata={(metadata) => protein.setMetadata(testRawMetadata)}/></Router>);

The error I am getting is this:

TypeError: (0 , _RestApi.getDataFromApi)(...).then(...).catch(...).finally is not a function

      65 |     this.cancelTokenSource = axios.CancelToken.source();
      66 |     const url = this.props["get-metadata-url"](query, organism);
    > 67 |     getDataFromApi([url], { cancelToken: this.cancelTokenSource.token })
         |     ^
      68 |       .then(response => {
      69 |         this.props["format-metadata"](response.data, organism);
      70 |       })

In any case, I am wondering if there is a better approach to testing a method like this.

jonrkarr commented 4 years ago

There are multiple ways to handle this:

In the branch, you have partly applied option 1, but you need to go further as I did for the data table. I will illustrate how this can be done in the next commit.

jonrkarr commented 4 years ago

I have re-factored the metadata sections so that the code which processes and formats the metadata sections does not need to access the properties or state of any component. This makes it easy to test using option 1 above. This also has the advantages of avoiding repeated CSS formatting for each metadata section and better integrating the metadata sections with the table of content.

You had partially employed this strategy, but it needed to be taken further. Also, static functions can be defined within classes. It is not necessary to define functions outside of classes to make them static.

See the revised code. With my changes, it should now be easy to use option 1.

Going forward, please do not approve your own pull requests. When you are ready for review, ask me to review the request. This will give us the opportunity to give you pointers as above.

jonrkarr commented 4 years ago

I have also now made all of the methods in the metadatasection classes static. I hadn't done this previously because two of the Protein/AbundanceDataTable methods (which serve similar purposes) still need to be instance methods. Once the accessing of taxonomic distance information is refactored, those methods can be changed to static methods.