-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Hide feature metadata barplot controls if no feature metadata is available; add first barplot panel tests; abstract Empress initialization code for JS testing #320
Merged
Merged
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
68e2edf
TST: Add initial barplot panel handler tests
fedarko f4ab799
TST: abstract tst Empress init code; add bpph tsts
fedarko a210f02
Merge branch 'master' of https://github.com/biocore/empress into no-f…
fedarko 58ce1e7
TST: make the broken test less broken
fedarko dda82fe
TST: get remaining planned barplot test working
fedarko 4bdf664
STY: prettify
fedarko File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,123 @@ | ||
| require(["jquery", "Empress", "UtilitiesForTesting"], function ( | ||
| $, | ||
| Empress, | ||
| UtilitiesForTesting | ||
| ) { | ||
| module("Barplot Panel Handler", { | ||
| setup: function () { | ||
| this.testData = UtilitiesForTesting.getTestData(); | ||
| }, | ||
| }); | ||
|
|
||
| test("Layout availability toggling: initialization", function () { | ||
| // The default layout should influence whether the barplot panel's | ||
| // "available" or "unavailable" content is shown. First, let's try the | ||
| // unrooted layout, which doesn't support barplots. | ||
| var empress = new Empress( | ||
| this.testData.tree, | ||
| this.testData.treeData, | ||
| this.testData.tdToInd, | ||
| this.testData.nameToKeys, | ||
| this.testData.layoutToCoordSuffix, | ||
| "Unrooted", | ||
| this.testData.yrscf, | ||
| this.testData.biom, | ||
| this.testData.fmCols, | ||
| this.testData.tm, | ||
| this.testData.im, | ||
| this.testData.canvas | ||
| ); | ||
| ok(empress._barplotPanel.availContent.classList.contains("hidden")); | ||
| notOk( | ||
| empress._barplotPanel.unavailContent.classList.contains("hidden") | ||
| ); | ||
|
|
||
| // And now let's try starting out in the rectangular layout, which | ||
| // *does* support barplots. | ||
| var empress2 = new Empress( | ||
| this.testData.tree, | ||
| this.testData.treeData, | ||
| this.testData.tdToInd, | ||
| this.testData.nameToKeys, | ||
| this.testData.layoutToCoordSuffix, | ||
| "Rectangular", | ||
| this.testData.yrscf, | ||
| this.testData.biom, | ||
| this.testData.fmCols, | ||
| this.testData.tm, | ||
| this.testData.im, | ||
| this.testData.canvas | ||
| ); | ||
| notOk(empress2._barplotPanel.availContent.classList.contains("hidden")); | ||
| ok(empress2._barplotPanel.unavailContent.classList.contains("hidden")); | ||
| }); | ||
| // test("Layout availability toggling post-initialization", function () { | ||
| // var empress = new Empress( | ||
| // this.testData.tree, | ||
| // this.testData.treeData, | ||
| // this.testData.tdToInd, | ||
| // this.testData.nameToKeys, | ||
| // this.testData.layoutToCoordSuffix, | ||
| // "Unrooted", | ||
| // this.testData.yrscf, | ||
| // this.testData.biom, | ||
| // this.testData.fmCols, | ||
| // this.testData.tm, | ||
| // this.testData.im, | ||
| // this.testData.canvas | ||
| // ); | ||
|
|
||
| // // After updating the layout to something that supports barplots, the | ||
| // // barplot "available content" should now be shown. | ||
| // // | ||
| // // NOTE: This fails in testing -- I get the following error: | ||
| // // TypeError: Cannot set property '0' of undefined | ||
| // // at Object.u (file:///home/marcus/Dropbox/Work/KnightLab/Empress/fedarko/empress/empress/support_files/vendor/gl-matrix.min.js:28:30362) | ||
| // // at Drawer.centerCameraOn (file:///home/marcus/Dropbox/Work/KnightLab/Empress/fedarko/empress/empress/support_files/js/drawer.js:466:17) | ||
| // // at Empress.centerLayoutAvgPoint (file:///home/marcus/Dropbox/Work/KnightLab/Empress/fedarko/empress/empress/support_files/js/empress.js:1929:22) | ||
| // // at Empress.updateLayout (file:///home/marcus/Dropbox/Work/KnightLab/Empress/fedarko/empress/empress/support_files/js/empress.js:1816:22) | ||
| // // at Object.<anonymous> (file:///home/marcus/Dropbox/Work/KnightLab/Empress/fedarko/empress/tests/test-barplot-panel-handler.js:68:17) | ||
| // empress.updateLayout("Rectangular"); | ||
| // notOk(empress._barplotPanel.availContent.classList.contains("hidden")); | ||
| // ok(empress._barplotPanel.unavailContent.classList.contains("hidden")); | ||
|
|
||
| // // ... And going back to a not-compatible-with-barplots layout should | ||
| // // switch back to the unavailable content. | ||
| // empress.updateLayout("Unrooted"); | ||
| // ok(empress._barplotPanel.availContent.classList.contains("hidden")); | ||
| // notOk(empress._barplotPanel.unavailContent.classList.contains("hidden")); | ||
| // }); | ||
| test("Barplot layers default to feature metadata layers, but only if feature metadata is available", function () { | ||
| var empressWithFM = new Empress( | ||
| this.testData.tree, | ||
| this.testData.treeData, | ||
| this.testData.tdToInd, | ||
| this.testData.nameToKeys, | ||
| this.testData.layoutToCoordSuffix, | ||
| "Unrooted", | ||
| this.testData.yrscf, | ||
| this.testData.biom, | ||
| this.testData.fmCols, | ||
| this.testData.tm, | ||
| this.testData.im, | ||
| this.testData.canvas | ||
| ); | ||
| equal(empressWithFM._barplotPanel.layers[0].barplotType, "fm"); | ||
|
|
||
| var empressWithNoFM = new Empress( | ||
| this.testData.tree, | ||
| this.testData.treeData, | ||
| this.testData.tdToInd, | ||
| this.testData.nameToKeys, | ||
| this.testData.layoutToCoordSuffix, | ||
| "Unrooted", | ||
| this.testData.yrscf, | ||
| this.testData.biom, | ||
| [], | ||
| {}, | ||
| {}, | ||
| this.testData.canvas | ||
| ); | ||
| equal(empressWithNoFM._barplotPanel.layers[0].barplotType, "sm"); | ||
| }); | ||
| }); | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error is caused by line 453 in drawer because treeSpaceCenterX and treeSpaceCenterY are defined by taking the offsetWidth/offsetHeight of treeContainer which is an html component that is not defined in the test. So, when glMatrix tries to multiply on line 465, it cant because centerMat has undefined entries. You should be able to fix this by adding an div with the id 'tree-container' to utilities-for-testing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe this is the problem -- a div with the ID
tree-containeris already defined intests/index.html, and it has offset width / height properties (document.getElementById("tree-container").offsetWidth;produces a number when run in the console oftests/index.html).Did some digging and I think I've managed to solve the problem (this test works now, at least). the problem was that
empress.initialize()(which callsdrawer.initialize()) wasn't being called anywhere (in the non-test Empress code it's only called in theempress-template.htmlfile). Took care of this by calling the function within the test, but then more errors came up -- but they boiled down to other HTML elements on the page not existing, like you said. I added for the missing elements and now this test works??? (edit: I realize now that the test-empress.js file already initialized just the drawer, so these changes may have been overkill... ah well things work at least :)The testing HTML is getting kind of bloated so we may want to try to auto-populate it in the future... but for now the tests work at least :P
(As a nice side note, I think this should make testing select-node-menu / canvasEvents / drawer more feasible down the line)