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

Bar chart e2e test #5220

Closed
wants to merge 11 commits into from
Closed

Bar chart e2e test #5220

wants to merge 11 commits into from

Conversation

rafawendel
Copy link
Member

@rafawendel rafawendel commented Oct 19, 2020

What type of PR is this?

  • Other

Description

According to the checklist proposed on issue #5208, adding chart end-to-end tests with Cypress is top priority test-wise.

This is a boilerplate of test that covers making a query and using it to generate a bar chart. The test will verify all tabs of VisualizationEditor and then plot a chart and verify that it is plotted by Plotly.

Todos:

  • Create boilerplate
  • Add prints
  • Add update test
  • Create final snapshot

Copy link
Member

@gabrieldutra gabrieldutra left a comment

Choose a reason for hiding this comment

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

Although it's good to check the chart exists in the Dialog, this doesn't get us as much value as the Percy snapshots (they track both rendering differences and the non-existence of the Charts). So for this I was thinking about having the createChartThroughUI able to create all sorts of charts and having this spec like this:

function createChartThroughUI(visualizationName, extraStepsFunction = () => {}) {
  // basic Chart setup with shared steps
  extraStepsFunction(); // may need better naming, but the idea is to configure extra options and assertions here
  // assert chart is rendered in the Dialog preview
  // save it
  return visualizationId; // this will be useful to later build a dashboard
}

it("creates Bar charts", function() {
  const basicBarChart = createChartThroughUI("Basic Bar Chart");
  const barChartWithSwappedAxes = createChartThroughUI("Bar Chart with Swapped Axes", () => {
    // swap axes
    // any extra ui assertions, e.g.: check it correctly swaps the Labels in the editor
  });
  // any other Bar Chart variations we want to cover with testing

  createDashboardWithVisualizations([basicBarChart, barChartWithSwappedAxes]).then(dashboardUrl => {
    cy.visit(dashboardUrl);
    cy.percySnapshot("Bar Chart");
  });
})

For building a dashboard with multiple visualizations there is an example with the Pivot table:

image

I'm not sure if the code is perfect to be reused, but I'd love to help with that as I think building dashboards with different configured visualizations are a way to cover many things at once 🙂.

LMK your opinion

Comment on lines 86 to 90
cy.getByTestId("VisualizationPreview")
.find("g.plot")
.should("exist")
.find("g.points")
.should("not.exist");
Copy link
Member

Choose a reason for hiding this comment

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

As we are doing this multiple times, let's create a helper assertChartExistsInPreview

@restyled-io restyled-io bot mentioned this pull request Nov 7, 2020
@rafawendel
Copy link
Member Author

Well I found out the trouble was that my loop would fill the array after it ran (because the push ran inside a .then). I read about this class of problems in Cypress and there doesn't seem to be a good solution. The cy.all Gabriel added some time ago shares the same limitations.

What I did at the end of the day was combining as and get statements to achieve the desired asynchronous behavior. It's quite cumbersome to use, but it is what it is.

@rafawendel rafawendel closed this Nov 20, 2020
@rafawendel rafawendel deleted the line-e2e-test branch November 20, 2020 23:36
@rafawendel rafawendel restored the line-e2e-test branch November 20, 2020 23:40
@rafawendel rafawendel reopened this Nov 20, 2020
@rafawendel rafawendel closed this Nov 20, 2020
@rafawendel rafawendel mentioned this pull request Nov 20, 2020
1 task
@rafawendel rafawendel deleted the line-e2e-test branch November 20, 2020 23:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants