Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
134 changes: 65 additions & 69 deletions packages/app/cypress/e2e/integration/settings.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,95 +8,91 @@ describe('App: Settings', { viewportWidth: 600 }, () => {
cy.startAppServer('e2e')
})

it('displays the settings for the current project', () => {
it('visits settings page', () => {
cy.visitApp()
cy.get('[href="#/settings"]').click()
cy.findByText('Project Settings').click()
cy.findByText('Settings').click()

cy.get('[data-cy="settings-projectId"]')

.findByText('abc123')
.should('be.visible')

cy.get('[data-cy="settings-config"]')
.scrollIntoView()
.should('be.visible')
.contains('animationDistanceThreshold')
cy.get('div[data-testid="header-bar"]').should('contain', 'Settings')
cy.findByText('Device Settings').should('be.visible')
cy.findByText('Project Settings').should('be.visible')
})

it('displays the settings for the current device', () => {
cy.visitApp()
cy.get('[href="#/settings"]').click()
cy.findByText('Device Settings').click()

cy.findByText('External Editor').should('be.visible')
cy.findByText('Testing Preferences').should('be.visible')
cy.findByText('Proxy Settings').should('be.visible')
})
it('can reconfigure a project', () => {
cy.visitApp('#settings')

it('calls a reconfigure mutation when click on the footer button', () => {
cy.visitApp()
cy.get('[href="#/settings"]').click()
cy.intercept('mutation-SettingsContainer_ReconfigureProject', { 'data': { 'reconfigureProject': true } }).as('ReconfigureProject')
cy.findByText('Reconfigure Project').click()
cy.wait('@ReconfigureProject')
})

it('selects well known editor', () => {
cy.withCtx(async (ctx) => {
ctx.coreData.localSettings.availableEditors = [
...ctx.coreData.localSettings.availableEditors,
// don't rely on CI machines to have specific editors installed
// so just adding one here
{
id: 'well-known-editor',
binary: '/usr/bin/well-known',
name: 'Well known editor',
},
]

ctx.coreData.localSettings.preferences.preferredEditorBinary = undefined
describe('external editor', () => {
beforeEach(() => {
cy.withCtx(async (ctx) => {
ctx.coreData.localSettings.availableEditors = [
...ctx.coreData.localSettings.availableEditors,
// don't rely on CI machines to have specific editors installed
// so just adding one here
{
id: 'well-known-editor',
binary: '/usr/bin/well-known',
name: 'Well known editor',
},
]

ctx.coreData.localSettings.preferences.preferredEditorBinary = undefined
})

cy.visitApp('#settings')
cy.contains('Device Settings').click()
})

cy.visitApp()
it('selects well known editor', () => {
cy.intercept('POST', 'mutation-ExternalEditorSettings_SetPreferredEditorBinary').as('SetPreferred')

cy.get('[href="#/settings"]').click()
cy.contains('Device Settings').click()
cy.contains('Choose your editor...').click()
cy.contains('Well known editor').click()
cy.wait('@SetPreferred').its('request.body.variables.value').should('include', '/usr/bin/well-known')

cy.intercept('POST', 'mutation-ExternalEditorSettings_SetPreferredEditorBinary').as('SetPreferred')
// doing invoke instead of `type` since `type` enters keys on-by-one, triggering a mutation
// for each keystroke, making it hard to intercept **only** the final request, which I want to
// assert contains `/usr/local/bin/vim'
cy.findByPlaceholderText('Custom path...').clear().invoke('val', '/usr/local/bin/vim').trigger('input').trigger('change')
cy.wait('@SetPreferred').its('request.body.variables.value').should('include', '/usr/local/bin/vim')
// navigate away and come back
// preferred editor selected from dropdown should have been persisted
cy.visitApp()
cy.get('[href="#/settings"]').click()
cy.wait(100)
cy.get('[data-cy="Device Settings"]').click()

cy.contains('Choose your editor...').click()
cy.contains('Well known editor').click()
cy.wait('@SetPreferred').its('request.body.variables.value').should('include', '/usr/bin/well-known')
cy.get('[data-cy="use-well-known-editor"]').should('be.checked')
cy.get('[data-cy="use-custom-editor"]').should('not.be.checked')
})

// navigate away and come back
// preferred editor selected from dropdown should have been persisted
cy.visitApp()
cy.get('[href="#/settings"]').click()
cy.wait(100)
cy.get('[data-cy="Device Settings"]').click()
it('allows custom editor', () => {
cy.intercept('POST', 'mutation-ExternalEditorSettings_SetPreferredEditorBinary').as('SetPreferred')

cy.get('[data-cy="use-well-known-editor"]').should('be.checked')
cy.get('[data-cy="use-custom-editor"]').should('not.be.checked')
// doing invoke instead of `type` since `type` enters keys on-by-one, triggering a mutation
// for each keystroke, making it hard to intercept **only** the final request, which I want to
// assert contains `/usr/local/bin/vim'
cy.findByPlaceholderText('Custom path...').clear().invoke('val', '/usr/local/bin/vim').trigger('input').trigger('change')
cy.wait('@SetPreferred').its('request.body.variables.value').should('include', '/usr/local/bin/vim')

// change to custom editor using input
cy.findByPlaceholderText('Custom path...').clear().invoke('val', '/usr/local/bin/vim').trigger('input').trigger('change')
// navigate away and come back
// preferred editor entered from input should have been persisted
cy.get('[href="#/settings"]').click()
cy.wait(100)
cy.get('[data-cy="Device Settings"]').click()

// navigate away and come back
// preferred editor entered from input should have been persisted
cy.get('[href="#/settings"]').click()
cy.wait(100)
cy.get('[data-cy="Device Settings"]').click()
cy.get('[data-cy="use-well-known-editor"]').should('not.be.checked')
cy.get('[data-cy="use-custom-editor"]').should('be.checked')
cy.get('[data-cy="custom-editor"]').should('have.value', '/usr/local/bin/vim')
})

it('lists file browser as available editor', () => {
cy.intercept('POST', 'mutation-ExternalEditorSettings_SetPreferredEditorBinary').as('SetPreferred')

cy.get('[data-cy="use-well-known-editor"]').should('not.be.checked')
cy.get('[data-cy="use-custom-editor"]').should('be.checked')
cy.contains('Choose your editor...').click()
Copy link
Contributor

Choose a reason for hiding this comment

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

We should use the defaultMessages when we can i.e.

import defaultMessages from '@packages/frontend-shared/src/locales/en-US.json'
cy.contains(defaultMessages.settingsPage.editor.noEditorSelectedPlaceholder).click()

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 is something I always struggle with when writing tests - it feels strange to be importing values to test against.

On the one hand, there's less duplication of strings, and if the string changes, then we don't have to update the test. We try to avoid embedding strings in code for good reasons, and some of those reasons apply to embedding strings in tests too.

On the other hand, if the string changes... the behavior has changed and wouldn't we want the test to point that out? If I replaced noEditorSelectedPlaceholder with an empty string in the localization, the test would still pass despite not showing what we expected to the user.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's a fair point. Plus, it makes the test a bit harder to read since I don't know exactly what DOM element it's trying to find, I either have to wait for the test to run or dig through the json. I think this is a good point to bring up to the team since we aren't consistent either way

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with @BlueWinds - I don't think it makes sense to assert against the same thing we test against. You could changing the string in en-US.json to some random junk and it would still pass.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For anyone following along at home, we've started a discussion in Slack about this, currently waiting for Jess to return and give her opinion on the matter before deciding which direction to go.

cy.get('[data-testid="computer"]').click()
Copy link
Contributor

Choose a reason for hiding this comment

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

I started a discussion in Slack regarding the test attributes in use today: data-cy, data-testid, and data-test. There is a desire to standardize on data-cy given that is what is in our wheelhouse and what we recommend to our users.

The biggest hurdle to that is our usage of testing-library's Cypress command extensions, but we could override the attribute they're looking for to data-cy and continue to use the various ByTestId lookups.


// change to custom editor using input
cy.get('[data-cy="custom-editor"]').should('have.value', '/usr/local/bin/vim')
cy.wait('@SetPreferred').its('request.body.variables.value').should('include', 'computer')
cy.get('[data-cy="use-well-known-editor"]').should('be.checked')
cy.get('[data-cy="use-custom-editor"]').should('not.be.checked')
})
})
})
28 changes: 21 additions & 7 deletions packages/app/src/settings/SettingsContainer.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,32 @@ import { defaultMessages } from '@cy/i18n'
import SettingsContainer from './SettingsContainer.vue'

describe('<SettingsContainer />', { viewportHeight: 800, viewportWidth: 900 }, () => {
it('renders', () => {
beforeEach(() => {
cy.mountFragment(SettingsContainerFragmentDoc, { render: (gql) => <SettingsContainer gql={gql} /> })
})

it('expands and collapses device settings', () => {
cy.contains('Device Settings').click()

cy.findByText(defaultMessages.settingsPage.editor.title).should('be.visible')
cy.findByText(defaultMessages.settingsPage.proxy.title).should('be.visible')
cy.findByText(defaultMessages.settingsPage.testingPreferences.title).should('be.visible')

cy.findByText('Device Settings').click()

cy.findByText(defaultMessages.settingsPage.editor.title).should('not.exist')
})

it('expands and collapses project settings', () => {
cy.contains('Project Settings').click()

cy.findByText(defaultMessages.settingsPage.projectId.title).should('be.visible')
cy.findByText(defaultMessages.settingsPage.config.title).should('be.visible').click()
})
cy.findByText(defaultMessages.settingsPage.experiments.title).should('be.visible')
cy.findByText(defaultMessages.settingsPage.specPattern.title).should('be.visible')
cy.findByText(defaultMessages.settingsPage.config.title).should('be.visible')

it('renders a footer', () => {
cy.mountFragment(SettingsContainerFragmentDoc, { render: (gql) => <SettingsContainer gql={gql} /> })
cy.findByText(defaultMessages.settingsPage.footer.text)
cy.findByText(defaultMessages.settingsPage.footer.button)
cy.findByText('Project Settings').click()

cy.findByText(defaultMessages.settingsPage.projectId.title).should('not.exist')
})
})
25 changes: 25 additions & 0 deletions packages/frontend-shared/src/components/CopyButton.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import CopyButton from './CopyButton.vue'

describe('<CopyButton />', { viewportHeight: 450, viewportWidth: 350 }, () => {
it('copies text to clipboard', () => {
cy.mount(() => (<>
<CopyButton text="Foobar" />
</>))
.get('button')
.should('contain.text', 'Copy')

.get('svg')
.should('exist')

// This button is broken on Firefox, but works properly on Chromium/Chrome/Electron
// TODO: Add assertions about actually clicking the button.
})

it('noIcon hides the icon', () => {
cy.mount(() => (<>
<CopyButton text="Foobar" noIcon={true} />
</>))
.get('svg')
.should('not.exist')
})
})
1 change: 1 addition & 0 deletions packages/frontend-shared/src/components/Select.vue
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@
'text-gray-800': !option.isSelected && !active,
'text-opacity-40': option.disabled || false
}]"
:data-testid="get(option, itemKey)"
>
<span class="flex inset-y-0 absolute items-center">
<slot
Expand Down