Skip to content

Commit

Permalink
fix: throw error so test failures are not swallowed (#17926)
Browse files Browse the repository at this point in the history
So obvious in retrospect

If you listen to the Cypress fail event without re-throwing then you swallow all test failures

"fun"

When reviewing #17919 I knew the Cypress tests would have to be failing which is what prompted me to check

Introduced in bbb7ed9 (July 10th!)
  • Loading branch information
pauldambra authored Oct 11, 2023
1 parent 1f9b42c commit 5da50e6
Show file tree
Hide file tree
Showing 22 changed files with 245 additions and 125 deletions.
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')
})
})
11 changes: 11 additions & 0 deletions cypress/e2e/insights-unsaved-confirmation.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ describe('Insights', () => {
)
)

// set window:confirm here to ensure previous tests can't block
cy.on('window:confirm', () => {
return true
})

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

Expand Down Expand Up @@ -44,10 +49,13 @@ describe('Insights', () => {
})

insight.newInsight()

cy.log('Add series')
cy.get('[data-attr=add-action-event-button]').click()

cy.log('Navigate away')
cy.get('[data-attr="menu-item-featureflags"]').click()

cy.log('Save button should still be here because case 1 rejects confirm()')
cy.get('[data-attr="insight-save-button"]').should('exist')
})
Expand All @@ -56,9 +64,12 @@ describe('Insights', () => {
cy.on('window:confirm', () => {
return true
})

insight.newInsight()

cy.log('Add series')
cy.get('[data-attr=add-action-event-button]').click()

cy.log('Navigate away')
cy.get('[data-attr="menu-item-featureflags"]').click()
cy.url().should('include', '/feature_flags')
Expand Down
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
22 changes: 12 additions & 10 deletions cypress/e2e/notebooks.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@ describe('Notebooks', () => {

cy.fixture('api/notebooks/notebook.json').then((notebook) => {
cy.intercept('GET', /api\/projects\/\d+\/notebooks\/.*\//, { body: notebook }).as('loadNotebook')
// this means saving doesn't work but so what?
cy.intercept('PATCH', /api\/projects\/\d+\/notebooks\/.*\//, (req, res) => {
res.reply(req.body)
// bounce the notebook patch back as if it succeeded,
// this means saving doesn't work in Cypress but so what?
cy.intercept('PATCH', /api\/projects\/\d+\/notebooks\/.*\//, (req) => {
req.reply(req.body)
}).as('patchNotebook')
})

Expand All @@ -42,19 +43,20 @@ describe('Notebooks', () => {

it('Insertion suggestions can be dismissed', () => {
cy.visit(urls.notebook('h11RoiwV'))
cy.get('.node-ph-replay-timestamp').click()
cy.get('.NotebookEditor').type('{enter}')

cy.get('.NotebookRecordingTimestamp--preview').should('exist')
cy.get('.NotebookRecordingTimestamp.opacity-50').should('exist')

cy.get('.NotebookEditor').type('{esc}')
cy.get('.NotebookFloatingButton .LemonButton').should('exist')
cy.get('.NotebookRecordingTimestamp.opacity-50').should('not.exist')
})

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 All @@ -66,8 +68,8 @@ describe('Notebooks', () => {
cy.get('li').contains('Notebooks').should('exist').click()
cy.get('[data-attr="new-notebook"]').click()
// we don't actually get a new notebook because the API is mocked
// so, "exit" the timestamp block we start in
cy.get('.NotebookEditor').type('{esc}{enter}{enter}')
// so, we need to clear the text
cy.get('.NotebookEditor').type('{selectAll}{backSpace}{enter}')
})

it('Can add a number list', () => {
Expand All @@ -84,7 +86,7 @@ describe('Notebooks', () => {

it('Can add bold', () => {
cy.get('.NotebookEditor').type('**bold**')
cy.get('.NotebookEditor p').last().should('contain.html', '<strong>bold</strong>')
cy.get('.NotebookEditor p').first().should('contain.html', '<strong>bold</strong>')
})

it('Can add bullet list', () => {
Expand Down
24 changes: 16 additions & 8 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 @@ -75,19 +75,27 @@ describe('Signup', () => {
})

it('Shows redirect notice if redirecting for maintenance', () => {
cy.visit('/logout')
cy.location('pathname').should('include', '/login')
cy.intercept('https://app.posthog.com/decide/*', (req) =>
req.reply(
decideResponse({
'redirect-signups-to-instance': 'us',
})
)
)
cy.visit('/signup?maintenanceRedirect=true')
cy.get('.Toastify__toast-body').should(
'contain',
`You have been redirected to signup on our US instance while we perform maintenance on our other instance.`
)

cy.visit('/logout')
cy.location('pathname').should('include', '/login')

cy.visit('/signup?maintenanceRedirect=true', {
onLoad(win: Cypress.AUTWindow) {
win.POSTHOG_APP_CONTEXT.preflight.cloud = true
},
})

cy.get('[data-attr="info-toast"]')
.contains(
`You've been redirected to signup on our US instance while we perform maintenance on our other instance.`
)
.should('be.visible')
})
})
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
5 changes: 3 additions & 2 deletions frontend/src/lib/components/Support/supportLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ function getSessionReplayLink(): string {

function getDjangoAdminLink(
user: UserType | null,
cloudRegion: Region | undefined,
cloudRegion: Region | null | undefined,
currentTeamId: TeamType['id'] | null
): string {
if (!user || !cloudRegion) {
Expand All @@ -33,7 +33,7 @@ function getDjangoAdminLink(
return `Admin: ${link} (Organization: '${user.organization?.name}'; Project: ${currentTeamId}:'${user.team?.name}')`
}

function getSentryLink(user: UserType | null, cloudRegion: Region | undefined): string {
function getSentryLink(user: UserType | null, cloudRegion: Region | null | undefined): string {
if (!user || !cloudRegion) {
return ''
}
Expand Down Expand Up @@ -209,6 +209,7 @@ export const supportLogic = kea<supportLogicType>([
zendesk_ticket_uuid +
')'
const cloudRegion = preflightLogic.values.preflight?.region

const payload = {
request: {
requester: { name: name, email: email },
Expand Down
4 changes: 4 additions & 0 deletions frontend/src/lib/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,10 @@ export function objectsEqual(obj1: any, obj2: any): boolean {
return equal(obj1, obj2)
}

export function isString(candidate: unknown): candidate is string {
return typeof candidate === 'string'
}

export function isObject(candidate: unknown): candidate is Record<string, unknown> {
return typeof candidate === 'object' && candidate !== null
}
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/queries/nodes/InsightViz/utils.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { ActionsNode, BreakdownFilter, EventsNode, InsightQueryNode, TrendsQuery } from '~/queries/schema'
import { ActionType, ChartDisplayType, InsightModel, IntervalType } from '~/types'
import { seriesToActionsAndEvents } from '../InsightQuery/utils/queryNodeToFilter'
import { getEventNamesForAction } from 'lib/utils'
import { getEventNamesForAction, isEmptyObject } from 'lib/utils'
import {
isInsightQueryWithBreakdown,
isInsightQueryWithSeries,
Expand Down Expand Up @@ -115,7 +115,7 @@ export const getCachedResults = (
cachedInsight: Partial<InsightModel> | undefined | null,
query: InsightQueryNode
): Partial<InsightModel> | undefined => {
if (!cachedInsight || cachedInsight.filters === undefined) {
if (!cachedInsight || cachedInsight.filters === undefined || isEmptyObject(cachedInsight.filters)) {
return undefined
}

Expand Down
Loading

0 comments on commit 5da50e6

Please sign in to comment.