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

fix: throw error so test failures are not swallowed #17926

Merged
merged 15 commits into from
Oct 11, 2023
1 change: 1 addition & 0 deletions .github/workflows/ci-e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ jobs:
# version changes
- docker-compose.dev.yml
- Dockerfile
- cypress/**

# Job that lists and chunks spec file names and caches node modules
chunks:
Expand Down
2 changes: 1 addition & 1 deletion cypress/e2e/a11y.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe('a11y', () => {
'persons',
'cohorts',
'annotations',
'plugins',
'apps',
'toolbarlaunch',
'projectsettings',
]
Expand Down
2 changes: 2 additions & 0 deletions cypress/e2e/dashboard-duplication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ describe('duplicating dashboards', () => {
let dashboardName, insightName, expectedCopiedDashboardName, expectedCopiedInsightName

beforeEach(() => {
cy.intercept('POST', /\/api\/projects\/\d+\/dashboards/).as('createDashboard')

dashboardName = randomString('dashboard-')
expectedCopiedDashboardName = `${dashboardName} (Copy)`

Expand Down
8 changes: 6 additions & 2 deletions cypress/e2e/events.cy.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { dayjs } from 'lib/dayjs'

const interceptPropertyDefinitions = (): void => {
cy.intercept('/api/event/values/?key=%24browser').as('getBrowserValues')

cy.intercept('api/projects/@current/property_definitions/?limit=5000', {
fixture: 'api/event/property_definitions',
})
Expand Down Expand Up @@ -71,8 +73,10 @@ describe('Events', () => {
cy.get('[data-attr=taxonomic-filter-searchfield]').click()
cy.get('[data-attr=prop-filter-event_properties-0]').click()
cy.get('[data-attr=prop-val] .ant-select-selector').click({ force: true })
cy.get('[data-attr=prop-val-0]').click()
cy.get('.DataTable').should('exist')
cy.wait('@getBrowserValues').then(() => {
cy.get('[data-attr=prop-val-0]').click()
cy.get('.DataTable').should('exist')
})
})

it('separates feature flag properties into their own tab', () => {
Expand Down
5 changes: 3 additions & 2 deletions cypress/e2e/featureFlags.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,13 @@ describe('Feature Flags', () => {

// Check that feature flags instructions can be displayed in another code language
cy.get('[data-attr=feature-flag-instructions-select]').click()
cy.get('[data-attr=feature-flag-instructions-select-option-PHP]').click()
// force click rather than scrolling the element into view
cy.get('[data-attr=feature-flag-instructions-select-option-php]').click({ force: true })
cy.get('[data-attr=feature-flag-instructions-snippet]').should(
'contain',
/if (PostHog::isFeatureEnabled('.*', 'some distinct id')) {/
)
cy.get('[data-attr=feature-flag-instructions-snippet]').should('contain', / \/\/ do something here/)
cy.get('[data-attr=feature-flag-instructions-snippet]').should('contain', / {4}\/\/ do something here/)
cy.get('[data-attr=feature-flag-instructions-snippet]').should('contain', /}/)
cy.get('[data-attr=feature-flag-doc-link]').should(
'have.attr',
Expand Down
20 changes: 18 additions & 2 deletions cypress/e2e/insights-date-picker.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,24 @@
import { decideResponse } from '../fixtures/api/decide'
import { urls } from 'scenes/urls'

describe('insights date picker', () => {
beforeEach(() => {
cy.intercept('https://app.posthog.com/decide/*', (req) =>
req.reply(
decideResponse({
hogql: true,
'data-exploration-insights': true,
})
)
)

cy.visit(urls.insightNew())
})

it('Can set the date filter and show the right grouping interval', () => {
cy.get('[data-attr=date-filter]').click()
cy.get('div').contains('Yesterday').should('exist').click()
cy.get('[data-attr=interval-filter]').should('contain', 'Hour')
cy.get('[data-attr=interval-filter] .LemonButton__content').should('contain', 'hour')
})

it('Can set a custom rolling date range', () => {
Expand All @@ -13,6 +29,6 @@ describe('insights date picker', () => {
cy.get('.RollingDateRangeFilter__label').should('contain', 'In the last').click()

// Test that the button shows the correct formatted range
cy.get('[data-attr=date-filter]').get('span').contains('Last 5 days').should('exist')
cy.get('[data-attr=date-filter]').get('.LemonButton__content').contains('Last 5 days').should('exist')
})
})
4 changes: 3 additions & 1 deletion cypress/e2e/insights.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,9 @@ describe('Insights', () => {
describe('view source', () => {
it('can open the query editor', () => {
insight.newInsight('TRENDS')
cy.get('[aria-label="View source (BETA)"]').click()
insight.save()
cy.get('[data-attr="more-button"]').click()
cy.get('[data-attr="show-insight-source"]').click()
cy.get('[data-attr="query-editor"]').should('exist')
})
})
Expand Down
4 changes: 2 additions & 2 deletions cypress/e2e/notebooks.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,9 @@ describe('Notebooks', () => {

it('Can comment on a recording', () => {
cy.visit(urls.replay())
cy.get('[data-attr="notebooks-replay-comment-button"]').click()
cy.get('[data-attr="notebooks-add-button"]').click()

cy.get('.LemonButton').contains('Comment in a new notebook').click()
cy.get('.LemonButton').contains('New notebook').click()

cy.get('.Notebook.Notebook--editable').should('be.visible')
cy.get('.ph-recording.NotebookNode').should('be.visible')
Expand Down
5 changes: 3 additions & 2 deletions cypress/e2e/signup.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ describe('Signup', () => {
cy.visit('/signup')
})

it('Cannot create acount with existing email', () => {
it('Cannot create account with existing email', () => {
cy.get('[data-attr=signup-email]').type('[email protected]').should('have.value', '[email protected]')
cy.get('[data-attr=password]').type('12345678').should('have.value', '12345678')
cy.get('[data-attr=signup-start]').click()
Expand Down Expand Up @@ -74,7 +74,8 @@ describe('Signup', () => {
cy.get('.Toastify [data-attr="error-toast"]').contains('Inactive social login session.')
})

it('Shows redirect notice if redirecting for maintenance', () => {
// skip this because it seems to be missing necessary setup feature flag, preflight cloud check...
it.skip('Shows redirect notice if redirecting for maintenance', () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

@raquelmsmith can this Cypress test be deleted or should it be fixed?

Copy link
Member

Choose a reason for hiding this comment

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

It shouldn't be deleted, we want to make sure this functionality works for when we do use the flag in production. It shouldn't be broken and was working before.. The bit we need for the flag is on line 83 below where we define what is in the decide response.

Copy link
Member Author

Choose a reason for hiding this comment

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

Just to check... did the test pass for you running locally before?

The problem we've had is that the tests were passing in CI no matter what. I've had to change the code a little...

  • I had to set preflight to cloud in the test itself
  • the redirect is hard-coded to use prod US or prod EU URLs so have had to change the redirect code to not redirect from Cypress to production

Copy link
Member

Choose a reason for hiding this comment

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

Yeah it did work locally! Thanks for the fix.

cy.visit('/logout')
cy.location('pathname').should('include', '/login')
cy.intercept('https://app.posthog.com/decide/*', (req) =>
Expand Down
12 changes: 8 additions & 4 deletions cypress/e2e/trends.cy.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,20 @@
import { urls } from 'scenes/urls'
import { insight, interceptInsightLoad } from '../productAnalytics'

describe('Trends', () => {
beforeEach(() => {
cy.visit(urls.insightNew())
insight.newInsight()
})

it('Can load a graph from a URL directly', () => {
const networkInterceptAlias = interceptInsightLoad('TRENDS')

// regression test, the graph wouldn't load when going directly to a URL
cy.visit(
'/insights/new?insight=TRENDS&interval=day&display=ActionsLineGraph&events=%5B%7B"id"%3A"%24pageview"%2C"name"%3A"%24pageview"%2C"type"%3A"events"%2C"order"%3A0%7D%5D&filter_test_accounts=false&breakdown=%24referrer&breakdown_type=event&properties=%5B%7B"key"%3A"%24current_url"%2C"value"%3A"http%3A%2F%2Fhogflix.com"%2C"operator"%3A"icontains"%2C"type"%3A"event"%7D%5D'
)

cy.wait(`@${networkInterceptAlias}`)

cy.get('[data-attr=trend-line-graph]').should('exist')
})

Expand Down Expand Up @@ -107,7 +111,7 @@ describe('Trends', () => {

it('Apply interval filter', () => {
cy.get('[data-attr=interval-filter]').click()
cy.contains('Week').click()
cy.contains('week').click()

cy.get('[data-attr=trend-line-graph]').should('exist')
})
Expand Down Expand Up @@ -136,7 +140,7 @@ describe('Trends', () => {
it('Apply date filter', () => {
cy.get('[data-attr=date-filter]').click()
cy.get('div').contains('Yesterday').should('exist').click()
cy.get('[data-attr=trend-line-graph]', { timeout: 10000 }).should('exist')
cy.get('.trends-insights-container .insight-empty-state').should('exist')
})

it('Apply property breakdown', () => {
Expand Down
2 changes: 1 addition & 1 deletion cypress/fixtures/api/event/property_definitions.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"results": [
{
"id": "017dde0e-1cb5-0000-68b4-44835b7c894f",
"name": "$browser_version",
"name": "$browser",
"is_numerical": true,
"query_usage_30_day": null,
"property_type": null,
Expand Down
4 changes: 2 additions & 2 deletions cypress/productAnalytics/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export const savedInsights = {
},
}

function interceptInsightLoad(insightType: string): string {
export function interceptInsightLoad(insightType: string): string {
cy.intercept('GET', /api\/projects\/\d+\/insights\/trend\/\?.*/).as('loadNewTrendsInsight')
cy.intercept('POST', /api\/projects\/\d+\/insights\/funnel\/?/).as('loadNewFunnelInsight')
cy.intercept('GET', /api\/projects\/\d+\/insights\/retention\/\?.*/).as('loadNewRetentionInsight')
Expand Down Expand Up @@ -116,7 +116,7 @@ export const insight = {
// force clicks rather than mess around scrolling rows that exist into view
cy.contains('button', 'Add to dashboard').click({ force: true })
cy.wait('@patchInsight').then(() => {
cy.contains('Added').should('exist')
cy.contains('Remove from dashboard').should('exist')
if (options?.visitAfterAdding) {
cy.contains('a', dashboardName).click({ force: true })
}
Expand Down
2 changes: 2 additions & 0 deletions cypress/support/e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { decideResponse } from '../fixtures/api/decide'
try {
// eslint-disable-next-line @typescript-eslint/no-var-requires
require('cypress-terminal-report/src/installLogsCollector')()
// eslint-disable-next-line no-empty
} catch {}

// Add console errors into cypress logs. This helps with failures in Github Actions which otherwise swallows them.
Expand Down Expand Up @@ -75,6 +76,7 @@ Cypress.on('fail', (error: Cypress.CypressError) => {
;(win as any).posthog?.capture('e2e_testing_test_failed', { e2e_testing_error_message: error.message })
})
}
throw error // so the test still fails
})

const resizeObserverLoopErrRe = /^[^(ResizeObserver loop limit exceeded)]/
Expand Down
3 changes: 3 additions & 0 deletions frontend/src/scenes/insights/InsightPageHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,9 @@ export function InsightPageHeader({ insightLogicProps }: { insightLogicProps: In
) : null}
{isInsightVizNode(query) ? (
<LemonButton
data-attr={`${
showQueryEditor ? 'hide' : 'show'
}-insight-source`}
status="stealth"
onClick={() => {
// for an existing insight in view mode
Expand Down