-
Notifications
You must be signed in to change notification settings - Fork 68
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
Update metric explorer tests so they work. #193
Conversation
- Add new test that creates and saves a chart - Update one of the test suites so that it actually checks that the charts are correct and runs consistently without having to add pause() calls - Also add a couple of id's to metric explorer menu objects to make testing easier. Note that the existing other tests have been left as is. These need to be improved.
@@ -4678,6 +4678,7 @@ Ext.extend(XDMoD.Module.MetricExplorer, XDMoD.PortalModule, { | |||
text: pmi, | |||
xtype: 'menuitem', | |||
menu: new Ext.menu.Menu({ | |||
id: 'me_new_chart_submenu_' + pmi, |
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.
is pmi
here always going to be a valid id (pretty much no spaces)?
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.
In the current code yes it will be since the different possible values are defined on line 4671 and they do not have spaces in them.
@@ -70,6 +132,39 @@ class MetricExplorer { | |||
.closest('table') | |||
.attr('id'); | |||
} | |||
createNewChart(chartName, datasetType, plotType) { | |||
browser.waitForChart(); |
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.
Should this be a waitForChartToChange
browser.waitUntilAnimEndAndClick(this.selectors.catalog.nodeByPath(realm, statistic)); | ||
browser.waitForVisible(this.selectors.catalog.addToChartMenu.container); | ||
browser.waitAndClick(this.selectors.catalog.addToChartMenu.itemByName(groupby)); | ||
browser.waitForChart(); |
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.
should this be waitForChartToChange
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.
No because it is possible to create a new chart even when there is no existing one. I double checked and waitForChart() actually waits for the chart loading mask to disappear. This is, obviously not a robust way of doing things since there is a race condition in the check running before the mask has been created, but I don't plan to fix this in this pull request.
Ensure that the autogenerated id does not contain spaces.
Description
charts are correct and runs consistently without having to add pause()
calls
testing easier.
Note that the existing other tests have been left as is. These need
to be improved.
Motivation and Context
testing
Tests performed
yes