Extended tests for manage_taxonomies.jsx file

[amir-qayyum-khan]

Hi

In this PR I wrote test cases for all reactjs classes i.e VocabularyComponent, TermComponent, AddTermsComponent, AddVocabulary, TaxonomyComponent

#322

@pdpinch @pwilkins @carsongee @noisecapella

[noisecapella]

There will be some changes coming in regarding the javascript tests so you will need to adjust this PR after my PR is reviewed and merged (#309). Hopefully we'll get a good process nailed down soon for javascript unit tests.

[noisecapella]

It looks like you could move this and updateParent inside the test where they are being used. In general we want to define variables as close to their use as possible

[noisecapella]

I also did some refactoring in PR #340 FYI, you may want to adjust your code accordingly. The most significant change is that state.terms was removed since it was a duplicate piece of state, a callback is used instead to update TaxonomyComponent

[noisecapella]

One more thing, I noticed that the vocabulary type radio box is not covered by tests, and that failures of various AJAX calls are not covered either. You can look at test_learning_resources.jsx for examples of this.

Thanks for all your help!

[amir-qayyum-khan]

@noisecapella please review, I think it is ready to merge

[noisecapella]

This variable is used in two tests. Please create a local variable within each test instead. We want to limit global state as much as possible, especially given the complexity of our test framework. The way it is now, one test may affect another tests result which will make test failures hard to debug.

[amir-qayyum-khan]

Done

[noisecapella]

You should move the rest of the code inside this function so that things are executed in proper order and done() is called only after everything else completes.

[noisecapella]

Looks good overall, just address the code coverage comment I made and move the code around to avoid race conditions as indicated in my comments, and this should be good to go

[amir-qayyum-khan]

Done @noisecapella

[noisecapella]

Unchecking the learning resource type checkbox is still uncovered. You should be able to test this case by just unchecking it with Simulate.change, use forceUpdate to wait for state change and verify that this.state.learningResourceTypes.length is 0

[amir-qayyum-khan]

@noisecapella which test you are referring to, please tell me. I have added this check here https://github.com/mitodl/lore/pull/328/files#diff-0f3ee7bcc78eb4497e0092ea2083d2e5R801 for TaxonomyComponent

[amir-qayyum-khan]

@noisecapella also i am testing it is AddVocabulary
https://github.com/mitodl/lore/pull/328/files#diff-0f3ee7bcc78eb4497e0092ea2083d2e5R520

[noisecapella]

I see the confusion, state.learningResourceTypes in TaxonomyComponent is a list of all available learning resource types but state.learningResourceTypes in AddVocabulary is a list of the currently checked learning resource types. You need to uncheck a learning resource type checkbox then verify that it is unchecked in state.learningResourceTypes in AddVocabulary.

You currently only check checkboxes which are previously unchecked so the code path that unchecks checkboxes is uncovered.

[amir-qayyum-khan]

got it

[amir-qayyum-khan]

Done @noisecapella

[noisecapella]

This is great, thanks for your help!