ktmud commented on a change in pull request #10158:
URL: 
https://github.com/apache/incubator-superset/pull/10158#discussion_r445695030



##########
File path: 
superset-frontend/cypress-base/cypress/integration/explore/visualizations/histogram.test.js
##########
@@ -0,0 +1,88 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+describe('Visualization > Histogram', () => {
+  const HISTOGRAM_FORM_DATA = {
+    datasource: '3__table',
+    viz_type: 'histogram',
+    slice_id: 60,
+    granularity_sqla: 'ds',
+    time_grain_sqla: 'P1D',
+    time_range: '100 years ago : now',
+    all_columns_x: ['num'],
+    adhoc_filters: [],
+    row_limit: 50000,
+    groupby: [],
+    color_scheme: 'bnbColors',
+    link_length: 5, // number of bins
+    x_axis_label: 'Frequency',
+    y_axis_label: 'Num',
+    global_opacity: 1,
+    normalized: false,
+  };
+
+  function verify(formData) {
+    cy.visitChartByParams(JSON.stringify(formData));
+    cy.verifySliceSuccess({ waitAlias: '@getJson', chartSelector: 'svg' });
+  }
+
+  beforeEach(() => {
+    cy.server();
+    cy.login();
+    cy.route('POST', '/superset/explore_json/**').as('getJson');
+  });
+
+  it('should work without groupby', () => {
+    verify(HISTOGRAM_FORM_DATA);
+    cy.get('.chart-container svg .vx-bar').should(
+      'have.length',
+      HISTOGRAM_FORM_DATA.link_length,

Review comment:
       Changed number of expected bin size.

##########
File path: 
superset-frontend/cypress-base/cypress/integration/explore/visualizations/time_table.js
##########
@@ -18,118 +18,115 @@
  */
 import { FORM_DATA_DEFAULTS, NUM_METRIC } from './shared.helper';

Review comment:
       The Time Table chart in testing is somehow empty. Leaving it as it for 
future persons who work on time table to debug.

##########
File path: 
superset-frontend/cypress-base/cypress/integration/explore/visualizations/sunburst.test.js
##########
@@ -0,0 +1,84 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+describe('Visualization > Sunburst', () => {
+  const SUNBURST_FORM_DATA = {
+    datasource: '2__table',
+    viz_type: 'sunburst',
+    slice_id: 47,
+    granularity_sqla: 'year',
+    time_grain_sqla: 'P1D',
+    time_range: 'No filter',
+    groupby: ['region'],
+    metric: 'sum__SP_POP_TOTL',
+    adhoc_filters: [],
+    row_limit: 50000,
+    color_scheme: 'bnbColors',
+  };
+
+  function verify(formData) {
+    cy.visitChartByParams(JSON.stringify(formData));
+    cy.verifySliceSuccess({ waitAlias: '@getJson', chartSelector: 'svg' });
+  }
+
+  beforeEach(() => {
+    cy.server();
+    cy.login();
+    cy.route('POST', '/superset/explore_json/**').as('getJson');
+  });
+
+  it('should work without secondary metric', () => {
+    verify(SUNBURST_FORM_DATA);
+    // There should be 7 visible arcs + 1 hidden
+    cy.get('.chart-container svg g#arcs path').should('have.length', 8);
+  });
+
+  it('should work with secondary metric', () => {
+    verify({
+      ...SUNBURST_FORM_DATA,
+      secondary_metric: 'sum__SP_RUR_TOTL',
+    });
+    cy.get('.chart-container svg g#arcs path').should('have.length', 8);
+  });
+
+  it('should work with multiple groupbys', () => {
+    verify({
+      ...SUNBURST_FORM_DATA,
+      groupby: ['region', 'country_name'],
+    });
+    cy.get('.chart-container svg g#arcs path').should('have.length', 117);

Review comment:
       Had to adjust some of the expected numbers because of library upgrades.

##########
File path: 
superset-frontend/cypress-base/cypress/integration/explore/visualizations/histogram.test.js
##########
@@ -0,0 +1,88 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+describe('Visualization > Histogram', () => {
+  const HISTOGRAM_FORM_DATA = {
+    datasource: '3__table',
+    viz_type: 'histogram',
+    slice_id: 60,
+    granularity_sqla: 'ds',
+    time_grain_sqla: 'P1D',
+    time_range: '100 years ago : now',
+    all_columns_x: ['num'],
+    adhoc_filters: [],
+    row_limit: 50000,
+    groupby: [],
+    color_scheme: 'bnbColors',
+    link_length: 5, // number of bins
+    x_axis_label: 'Frequency',
+    y_axis_label: 'Num',
+    global_opacity: 1,
+    normalized: false,
+  };
+
+  function verify(formData) {
+    cy.visitChartByParams(JSON.stringify(formData));
+    cy.verifySliceSuccess({ waitAlias: '@getJson', chartSelector: 'svg' });
+  }
+
+  beforeEach(() => {
+    cy.server();
+    cy.login();
+    cy.route('POST', '/superset/explore_json/**').as('getJson');
+  });
+
+  it('should work without groupby', () => {
+    verify(HISTOGRAM_FORM_DATA);
+    cy.get('.chart-container svg .vx-bar').should(
+      'have.length',
+      HISTOGRAM_FORM_DATA.link_length,
+    );
+  });
+
+  it('should work with group by', () => {
+    verify({
+      ...HISTOGRAM_FORM_DATA,
+      groupby: ['gender'],
+    });
+    cy.get('.chart-container svg .vx-bar').should(
+      'have.length',
+      HISTOGRAM_FORM_DATA.link_length * 2,
+    );
+  });
+
+  it('should work with filter and update num bins', () => {
+    const numBins = 2;

Review comment:
       When distribution is very long tail, VX histogram sometimes will 
generate bars < numBins. I use to very small `numBins` to pass the test.

##########
File path: 
superset-frontend/cypress-base/cypress/integration/explore/visualizations/pivot_table.test.js
##########
@@ -0,0 +1,107 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+describe('Visualization > Pivot Table', () => {
+  const PIVOT_TABLE_FORM_DATA = {
+    datasource: '3__table',
+    viz_type: 'pivot_table',
+    slice_id: 61,
+    granularity_sqla: 'ds',
+    time_grain_sqla: 'P1D',
+    time_range: '100+years+ago+:+now',
+    metrics: ['sum__num'],
+    adhoc_filters: [],
+    groupby: ['name'],
+    columns: ['state'],
+    row_limit: 50000,
+    pandas_aggfunc: 'sum',
+    pivot_margins: true,
+    number_format: '.3s',
+    combine_metric: false,
+  };
+
+  const TEST_METRIC = {
+    expressionType: 'SIMPLE',
+    column: {
+      id: 338,
+      column_name: 'sum_boys',
+      expression: '',
+      filterable: false,
+      groupby: false,
+      is_dttm: false,
+      type: 'BIGINT',
+      optionName: '_col_sum_boys',
+    },
+    aggregate: 'SUM',
+    hasCustomLabel: false,
+    label: 'SUM(sum_boys)',
+    optionName: 'metric_gvpdjt0v2qf_6hkf56o012',
+  };
+
+  function verify(formData) {
+    cy.visitChartByParams(JSON.stringify(formData));
+    cy.verifySliceSuccess({ waitAlias: '@getJson', chartSelector: 'table' });
+  }
+
+  beforeEach(() => {
+    cy.server();
+    cy.login();
+    cy.route('POST', '/superset/explore_json/**').as('getJson');
+  });
+
+  it('should work with single groupby', () => {
+    verify(PIVOT_TABLE_FORM_DATA);
+    cy.get('.chart-container tr:eq(0) th:eq(1)').contains('sum__num');
+    cy.get('.chart-container tr:eq(1) th:eq(0)').contains('state');
+    cy.get('.chart-container tr:eq(2) th:eq(0)').contains('name');

Review comment:
       Refactored how most pivot table assertions work. Previously it was 
something like
   
   ```js
         cy.get('.chart-container tr th').then(ths => {
           expect(ths.find(th => th.innerText.trim() === 'name')).to.not.equal(
             undefined,
           );
         });
   ```

##########
File path: 
superset-frontend/cypress-base/cypress/integration/explore/visualizations/treemap.test.js
##########
@@ -0,0 +1,84 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+describe('Visualization > Treemap', () => {
+  const TREEMAP_FORM_DATA = {
+    datasource: '2__table',
+    viz_type: 'treemap',
+    slice_id: 50,
+    granularity_sqla: 'year',
+    time_grain_sqla: 'P1D',
+    time_range: 'No filter',
+    metrics: ['sum__SP_POP_TOTL'],
+    adhoc_filters: [],
+    groupby: ['country_code'],
+    row_limit: 50000,
+    color_scheme: 'bnbColors',
+    treemap_ratio: 1.618033988749895,
+    number_format: '.3s',
+  };
+
+  const level0 = '.chart-container rect[style="fill: rgb(255, 90, 95);"]';
+  const level1 = '.chart-container rect[style="fill: rgb(123, 0, 81);"]';
+  const level2 = '.chart-container rect[style="fill: rgb(0, 122, 135);"]';

Review comment:
       Use color to detect lookup different levels of treemap rects. The tests 
might break if default color palettes updates, which is not ideal, but this is 
the only way I could find to find rect of different levels. The classnames used 
in previous tests have stopped working:
   
   ```js
         cy.get('.chart-container svg rect.parent').should('have.length', 7);
         cy.get('.chart-container svg rect.child').should('have.length', 214);
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to