Skip to content
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

Adding simple Cypress tests #5693

Merged
merged 3 commits into from
Sep 6, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ matrix:
env: TOXENV=py36-sqlite
- python: 3.6
env: TOXENV=pylint
- python: 3.6
env: TOXENV=cypress
exclude:
- python: 2.7
- python: 3.6
Expand Down
3 changes: 3 additions & 0 deletions superset/assets/cypress.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"baseUrl": "http://localhost:8081"
}
26 changes: 26 additions & 0 deletions superset/assets/cypress/integration/dashboard/dashboard_tests.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
describe('Load dashboard', function () {
it('Load birth names dashboard', function () {
cy.server();
cy.login();

cy.visit('/superset/dashboard/births');

cy.route('POST', '/superset/explore_json/**').as('getJson');
cy.wait(10000, ['@getJson']);

let sliceData;

cy.get('@getJson.all').then((xhrs) => {
sliceData = xhrs;
xhrs.forEach((data) => {
expect(data.status).to.eq(200);
expect(data.response.body).to.have.property('error', null);
cy.get(`#slice-container-${data.response.body.form_data.slice_id}`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this line do?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would verify that there exists a slice container in the dashboard for each slice loaded.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it for sure fail in the case that it doesn't exist? is it better/safer to chain a .then() with an expect?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes a get will fail with CypressError: Timed out retrying: Expected to find element the two expects above should always catch before the last one does, but seems good to have our bases covered.

});
cy.get('#app').then((data) => {
const bootstrapData = JSON.parse(data[0].dataset.bootstrap);
expect(bootstrapData.dashboard_data.slices.length).to.eq(sliceData.length);
});
});
});
});
59 changes: 59 additions & 0 deletions superset/assets/cypress/integration/explore/control_tests.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// ***********************************************
// Tests for setting controls in the UI
// ***********************************************

describe('Groupby', function () {
it('Set groupby', function () {
cy.server();
cy.login();

cy.route('POST', '/superset/explore_json/**').as('getJson');
cy.visitChartByName('Num Births Trend');
cy.verifySliceSuccess('@getJson');

cy.get('[data-test=groupby]').within(() => {
cy.get('.Select-control').click();
cy.get('input.select-input').type('state', { force: true });
cy.get('.VirtualizedSelectFocusedOption').click();
});
cy.get('button.query').click();
cy.verifySliceSuccess('@getJson');
});
});

describe('SimpleAdhocMetric', function () {
it('Clear metric and set simple adhoc metric', function () {
cy.server();
cy.login();

const metricName = 'Girl Births';

cy.route('POST', '/superset/explore_json/**').as('getJson');
cy.visitChartByName('Num Births Trend');
cy.verifySliceSuccess('@getJson');

cy.get('[data-test=metrics]').within(() => {
cy.get('.select-clear').click();
cy.get('.Select-control').click({ force: true });
cy.get('input').type('sum_girls', { force: true });
cy.get('.VirtualizedSelectFocusedOption').trigger('mousedown').click();
});

cy.get('#metrics-edit-popover').within(() => {
cy.get('.popover-title').within(() => {
cy.get('span').click();
cy.get('input').type(metricName);
});
cy.get('button').contains('Save').click();
});

cy.get('button.query').click();
cy.wait(['@getJson']).then((data) => {
expect(data.status).to.eq(200);
expect(data.response.body).to.have.property('error', null);
expect(data.response.body.data[0].key).to.equal(metricName);
cy.get('.slice_container');
});
});
});

54 changes: 54 additions & 0 deletions superset/assets/cypress/integration/explore/visualization_tests.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// ***********************************************
// Tests for visualization types
// ***********************************************

const FORM_DATA_DEFAULTS = {
datasource: '3__table',
viz_type: 'line',
granularity_sqla: 'ds',
time_grain_sqla: null,
time_range: '100+years+ago+:+now',
adhoc_filters: [],
groupby: [],
limit: null,
timeseries_limit_metric: null,
order_desc: false,
contribution: false,
};

describe('Line', function () {
it('Test line chart with adhoc metric', function () {
cy.server();
cy.login();

const metrics = [{
expressionType: 'SIMPLE',
column: {
id: 336,
column_name: 'num',
verbose_name: null,
description: null,
expression: '',
filterable: false,
groupby: false,
is_dttm: false,
type: 'BIGINT',
database_expression: null,
python_date_format: null,
optionName: '_col_num',
},
aggregate: 'SUM',
sqlExpression: null,
hasCustomLabel: false,
fromFormData: false,
label: 'SUM(num)',
optionName: 'metric_1de0s4viy5d_ly7y8k6ghvk',
}];

const formData = { ...FORM_DATA_DEFAULTS, metrics };

cy.route('POST', '/superset/explore_json/**').as('getJson');
cy.visitChartByParams(JSON.stringify(formData));
cy.verifySliceSuccess('@getJson');
});
});
17 changes: 17 additions & 0 deletions superset/assets/cypress/plugins/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// ***********************************************************
// This example plugins/index.js can be used to load plugins
//
// You can change the location of this file or turn off loading
// the plugins file with the 'pluginsFile' configuration option.
//
// You can read more here:
// https://on.cypress.io/plugins-guide
// ***********************************************************

// This function is called when a project is opened or re-opened (e.g. due to
// the project's config changing)

module.exports = (/* on, config */) => {
// `on` is used to hook into various events Cypress emits
// `config` is the resolved Cypress config
};
59 changes: 59 additions & 0 deletions superset/assets/cypress/support/commands.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// ***********************************************
// This example commands.js shows you how to
// create various custom commands and overwrite
// existing commands.
//
// For more comprehensive examples of custom
// commands please read more here:
// https://on.cypress.io/custom-commands
// ***********************************************
//
//
// -- This is a parent command --
// Cypress.Commands.add("login", (email, password) => { ... })
//
//
// -- This is a child command --
// Cypress.Commands.add("drag", { prevSubject: 'element'}, (subject, options) => { ... })
//
//
// -- This is a dual command --
// Cypress.Commands.add("dismiss", { prevSubject: 'optional'}, (subject, options) => { ... })
//
//
// -- This is will overwrite an existing command --
// Cypress.Commands.overwrite("visit", (originalFn, url, options) => { ... })

const BASE_EXPLORE_URL = '/superset/explore/?form_data=';

Cypress.Commands.add('login', () => {
cy.request({
method: 'POST',
url: 'http://localhost:8081/login/',
body: { username: 'admin', password: 'general' },
}).then((response) => {
expect(response.status).to.eq(200);
});
});

Cypress.Commands.add('visitChartByName', (name) => {
cy.request(`http://localhost:8081/chart/api/read?_flt_3_slice_name=${name}`).then((response) => {
cy.visit(`${BASE_EXPLORE_URL}{"slice_id": ${response.body.pks[0]}}`);
});
});

Cypress.Commands.add('visitChartById', (chartId) => {
cy.visit(`${BASE_EXPLORE_URL}{"slice_id": ${chartId}}`);
});

Cypress.Commands.add('visitChartByParams', (params) => {
cy.visit(`${BASE_EXPLORE_URL}${params}`);
});

Cypress.Commands.add('verifySliceSuccess', (waitAlias) => {
cy.wait([waitAlias]).then((data) => {
expect(data.status).to.eq(200);
expect(data.response.body).to.have.property('error', null);
cy.get('.slice_container');
});
});
20 changes: 20 additions & 0 deletions superset/assets/cypress/support/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// ***********************************************************
// This example support/index.js is processed and
// loaded automatically before your test files.
//
// This is a great place to put global configuration and
// behavior that modifies Cypress.
//
// You can change the location of this file or turn off
// automatically serving support files with the
// 'supportFile' configuration option.
//
// You can read more here:
// https://on.cypress.io/configuration
// ***********************************************************

// Import commands.js using ES2015 syntax:
import './commands';

// Alternatively you can use CommonJS syntax:
// require('./commands')
16 changes: 16 additions & 0 deletions superset/assets/cypress_build.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#!/bin/bash
set -e

superset/bin/superset db upgrade
superset/bin/superset load_test_users
superset/bin/superset load_examples
superset/bin/superset init
superset/bin/superset runserver &

cd "$(dirname "$0")"

npm install -g yarn
yarn
npm run build
npm run cypress run
kill %1
4 changes: 3 additions & 1 deletion superset/assets/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
"build": "webpack --mode=production --colors --progress",
"lint": "eslint --ignore-path=.eslintignore --ext .js,.jsx .",
"lint-fix": "eslint --fix --ignore-path=.eslintignore --ext .js,.jsx .",
"sync-backend": "babel-node --presets env src/syncBackend.js"
"sync-backend": "babel-node --presets env src/syncBackend.js",
"cypress": "cypress"
},
"repository": {
"type": "git",
Expand Down Expand Up @@ -141,6 +142,7 @@
"chai": "^4.0.2",
"clean-webpack-plugin": "^0.1.19",
"css-loader": "^0.28.0",
"cypress": "^3.0.3",
"enzyme": "^2.0.0",
"eslint": "^4.19.0",
"eslint-config-airbnb": "^15.0.1",
Expand Down
1 change: 1 addition & 0 deletions superset/assets/src/explore/components/Control.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ export default class Control extends React.PureComponent {
const divStyle = this.props.hidden ? { display: 'none' } : null;
return (
<div
data-test={this.props.name}
Copy link
Contributor

@williaster williaster Aug 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be removed? I think it's not ideal to have props set solely for tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was listed as a best practice by cypress to avoid using css selectors that may change https://docs.cypress.io/guides/references/best-practices.html#Selecting-Elements

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but we use css selectors elsewhere in these tests right? that seems unavoidable to some extent. If you feel strongly about it let's go with this pattern but perhaps you could set it to data-name so it indicates what the value is rather than just 'test'?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seemed ok to use some id / class selectors that are less likely to change. But when I google around for blogs discussing making UI tests resilient to change, this pattern comes up often.

This was a particular case where there was no identifier on the tag that was unique to it being a metric control (vs other type of control), so I needed to add some kind of id or identifier here, seemed good to be clear that it's just used for testing (so we know not to remove / change it). That's the reason I like using data_test, but I can change it if you feel strongly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it, thanks for more context. I'm cool with whatever you feel is best here 👍

style={divStyle}
onMouseEnter={this.setHover.bind(this, true)}
onMouseLeave={this.setHover.bind(this, false)}
Expand Down
Loading