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

🪟 🧪 E2E Tests for auto-detect schema changes #20682

Merged
merged 24 commits into from
Feb 2, 2023

Conversation

edmundito
Copy link
Contributor

@edmundito edmundito commented Dec 19, 2022

What

Closes #17936

Adds E2E tests for auto-detect schema changes:

  • Verify the status on the connection list page
  • Verify that the user can save through the breaking and non-breaking flow
  • Verify that the user can update the non-breaking preference

To make this possible, there were a few additions to the cypress framework:

  • Adds the ability to create sources, destinations, and connections without using the UI
  • Update the db query helpers to be able to generate SQL commands more easily

@octavia-squidington-iv octavia-squidington-iv added area/platform issues related to the platform area/frontend Related to the Airbyte webapp labels Dec 19, 2022
@edmundito edmundito marked this pull request as ready for review December 21, 2022 21:13
@edmundito edmundito requested a review from a team as a code owner December 21, 2022 21:13
@edmundito edmundito requested review from SofiiaZaitseva, dizel852 and krishnaglick and removed request for a team December 21, 2022 21:13
@edmundito edmundito changed the title [DRAFT] 🪟 🧪 E2E Tests for auto-detect schema changes 🪟 🧪 E2E Tests for auto-detect schema changes Dec 22, 2022
* Converts Cypress chain to regular Promise.
*/
export const toPromise = <T>(chain: Cypress.Chainable<T>): Promise<T> => {
return new Cypress.Promise((resolve, reject) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return new Cypress.Promise((resolve, reject) => {
return new Cypress.Promise<T>((resolve, reject) => {


export const clickSchemaChangesReviewButton = () => {
cy.get(schemaChangesReviewButton).click();
// cy.get(schemaChangesReviewButton).should("be.disabled");
Copy link
Contributor

Choose a reason for hiding this comment

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

Why commented?
We need to enable the new table?

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 related to not disabling the button in the loading state, which is fixed in #20737

@edmundito edmundito removed the request for review from SofiiaZaitseva January 12, 2023 15:01
@edmundito edmundito force-pushed the edmundito/auto-detect-schema-tests branch from 4924ed9 to 11a25db Compare January 18, 2023 21:43
@octavia-squidington-iv octavia-squidington-iv removed the area/platform issues related to the platform label Jan 18, 2023
Comment on lines +1 to +22
export const createTable = (tableName: string, columns: string[]): string =>
`CREATE TABLE ${tableName}(${columns.join(", ")});`;

export const dropTable = (tableName: string) => `DROP TABLE IF EXISTS ${tableName}`;

export const alterTable = (tableName: string, params: { add?: string[]; drop?: string[] }): string => {
const adds = params.add ? params.add.map((add) => `ADD COLUMN ${add}`) : [];
const drops = params.drop ? params.drop.map((columnName) => `DROP COLUMN ${columnName}`) : [];
const alterations = [...adds, ...drops];

return `ALTER TABLE ${tableName} ${alterations.join(", ")};`;
};

export const insertIntoTable = (tableName: string, valuesByColumn: Record<string, unknown>): string => {
const keys = Object.keys(valuesByColumn);
const values = keys
.map((key) => valuesByColumn[key])
.map((value) => (typeof value === "string" ? `'${value}'` : value));

return `INSERT INTO ${tableName}(${keys.join(", ")}) VALUES(${values.join(", ")});`;
};

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@dizel852 dizel852 left a comment

Choose a reason for hiding this comment

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

🔥 Happy to see so many good improvements - create a connection via API, instead of running the full flow of creation a new connection 👍

Also tried to run new tests locally, but wasn't successful. Fails on statusCell:
image

@edmundito edmundito force-pushed the edmundito/auto-detect-schema-tests branch from 11a25db to b80b47c Compare January 27, 2023 21:33
@edmundito edmundito force-pushed the edmundito/auto-detect-schema-tests branch from 45467f1 to e06f4f5 Compare January 30, 2023 19:57
@dizel852
Copy link
Contributor

dizel852 commented Feb 1, 2023

Tested locally, 1 test fails:
image

In the new table we use <SyncModeSelect />dropdown instead of the old one - <SyncSettingsDropdown />

@edmundito
Copy link
Contributor Author

@dizel852 this is currently not compatible with the new table.

Copy link
Contributor

@dizel852 dizel852 left a comment

Choose a reason for hiding this comment

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

Retested locally with old stream table - works like a charm 👍
Can't wait to start using the new test API 🔥

@edmundito edmundito merged commit c2890bf into master Feb 2, 2023
@edmundito edmundito deleted the edmundito/auto-detect-schema-tests branch February 2, 2023 15:09
letiescanciano added a commit that referenced this pull request Feb 6, 2023
* master: (86 commits)
  Discover worker starts to use API to write schema result (#21875)
  🪟 🎉  Connector Builder Landing Page (#22122)
  Fix pnpm cache path (#22418)
  Add additional shorter setup guides (#22318)
  Source Amazon Ads: fix reports stream records primary keys (#21677)
  Connector acceptance test: Fix discovered catalog caching for different configs (#22301)
  🪟🐛 Make modal scrollable (#21973)
  only compute diff if the schema discovery actually succeeded (#22377)
  Source Klaviyo: fix schema (#22071)
  🪟 🔧 Switch to `pnpm` for package managing (#22053)
  Source Sentry: turn on default availability strategy (#22303)
  Source freshdesk: deduplicate table names (#22164)
  Update connector-acceptance-tests-reference.md (#22370)
  Update the default security groups for the EC2 runner (#22347)
  Trace refresh schema operations (#22326)
  Remove manual docker upgrades from workflows (#22344)
  Update CODEOWNERS for connector acceptance tests to connector ops (#22341)
  🐛 source: airtable - handle singleSelect types (#22311)
  Source tiktok: chunk advertiser IDs (#22309)
  🪟 🧪 E2E Tests for auto-detect schema changes (#20682)
  ...
letiescanciano added a commit that referenced this pull request Feb 6, 2023
* master: (24 commits)
  Discover worker starts to use API to write schema result (#21875)
  🪟 🎉  Connector Builder Landing Page (#22122)
  Fix pnpm cache path (#22418)
  Add additional shorter setup guides (#22318)
  Source Amazon Ads: fix reports stream records primary keys (#21677)
  Connector acceptance test: Fix discovered catalog caching for different configs (#22301)
  🪟🐛 Make modal scrollable (#21973)
  only compute diff if the schema discovery actually succeeded (#22377)
  Source Klaviyo: fix schema (#22071)
  🪟 🔧 Switch to `pnpm` for package managing (#22053)
  Source Sentry: turn on default availability strategy (#22303)
  Source freshdesk: deduplicate table names (#22164)
  Update connector-acceptance-tests-reference.md (#22370)
  Update the default security groups for the EC2 runner (#22347)
  Trace refresh schema operations (#22326)
  Remove manual docker upgrades from workflows (#22344)
  Update CODEOWNERS for connector acceptance tests to connector ops (#22341)
  🐛 source: airtable - handle singleSelect types (#22311)
  Source tiktok: chunk advertiser IDs (#22309)
  🪟 🧪 E2E Tests for auto-detect schema changes (#20682)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/frontend Related to the Airbyte webapp
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add E2E tests for auto-detect schema changes frontend
4 participants