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: running a new test after already having run tests #21087

Merged
16 changes: 14 additions & 2 deletions packages/app/cypress/e2e/cypress-in-cypress-e2e.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,20 @@ describe('Cypress In Cypress E2E', { viewportWidth: 1500, defaultCommandTimeout:
cy.contains('switch spec')
cy.contains('withWait.spec').click()

cy.wait(5000)
cy.get('.passed > .num').should('contain', 4)
cy.get('.passed > .num', { timeout: 10000 }).should('contain', 4)
cy.get('.failed > .num').should('not.contain', 1)
})

it('executes a test, navigates back to the spec list, creates a new spec, and runs the new spec', () => {
cy.visitApp()
cy.contains('dom-content.spec').click()
cy.get('[data-model-state="passed"]').should('contain', 'renders the test content')
cy.contains('a', 'Specs').click()
cy.withCtx(async (ctx, o) => {
await ctx.actions.file.writeFileInProject(o.path, `describe('Simple Test', () => { it('true is true', () => { expect(true).to.be.true }) })`)
}, { path: getPathForPlatform('cypress/e2e/new-file.spec.js') })

cy.contains('new-file.spec').click()
cy.get('[data-model-state="passed"]').should('contain', 'expected true to be true')
})
})
2 changes: 1 addition & 1 deletion packages/app/cypress/e2e/sidebar_navigation.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ describe('Sidebar Navigation', () => {
context('as e2e testing type', () => {
beforeEach(() => {
cy.scaffoldProject('todos')
cy.scaffoldProject('pristine-with-e2e-testing')
cy.openProject('todos')
cy.startAppServer()
cy.visitApp()
Expand All @@ -51,6 +50,7 @@ describe('Sidebar Navigation', () => {

it('closes the left nav bar when clicking the expand button (if expanded)', () => {
cy.findByLabelText('Sidebar').closest('[aria-expanded]').should('have.attr', 'aria-expanded', 'true')
cy.contains('todos')
cy.findAllByText('todos').eq(1).as('title')
cy.get('@title').should('be.visible')

Expand Down
13 changes: 6 additions & 7 deletions packages/app/src/runner/SpecRunnerContainerOpenMode.vue
Original file line number Diff line number Diff line change
Expand Up @@ -22,24 +22,23 @@ const specStore = useSpecStore()
const router = useRouter()
const route = useRoute()

const { initialized, watchSpec } = useUnifiedRunner()

const specs = computed(() => {
return props.gql.currentProject?.specs ?? []
})

watchSpec(specs)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a strong opinion but I like to functions that call watch or watchEffect in the actual component file to give visibility, or you need to dig through a bunch of other files to find the watcher. It's like when you are trying to track down a useState or useEffect nested deeply in a hook in React.

I don't mind too much, but something about watchSpec(specs) is more readable to me than useUnifiedRunner(specs). The former is more clear that something reactive will happen. Happy to field opinions and not really a big deal 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'd prefer adding 'watcher' to the composable's name in some way if more discoverability is desired. Putting the responsibility of activating the watchers on the consuming component just leaves a logic hole where there doesn't need to be one.

const { initialized, watchSpecs } = useUnifiedRunner()

specStore.$subscribe((mutation, state) => {
const file = getPathForPlatform(route.query.file as string)
watchSpecs(specs)

const shouldRedirect = route.name === 'SpecRunner' && file && state.activeSpec === null
specStore.$subscribe((mutation, state) => {
const queryFile = getPathForPlatform(route.query.file as string)
const shouldRedirect = route.name === 'SpecRunner' && queryFile && state.activeSpec === null

if (shouldRedirect) {
router.push({
name: 'Specs',
params: {
unrunnable: file,
unrunnable: queryFile,
},
})
}
Expand Down
4 changes: 2 additions & 2 deletions packages/app/src/runner/SpecRunnerContainerRunMode.vue
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const props = defineProps<{

const specStore = useSpecStore()

const { initialized, watchSpec } = useUnifiedRunner()
const { initialized, watchSpecs } = useUnifiedRunner()

watchSpec(ref(props.runModeSpecs))
watchSpecs(ref(props.runModeSpecs))
</script>
30 changes: 12 additions & 18 deletions packages/app/src/runner/unifiedRunner.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import type { Ref } from 'vue'
import { onMounted, ref, watch, onBeforeUnmount, readonly } from 'vue'
import { useRoute } from 'vue-router'
import { Ref, onMounted, ref, watch, watchEffect, onBeforeUnmount, readonly } from 'vue'
import { getAutIframeModel, UnifiedRunnerAPI } from '../runner'
import { useSpecStore } from '../store'
import { useSelectorPlaygroundStore } from '../store/selector-playground-store'
import type { SpecFile } from '@packages/types/src'
import { useRoute } from 'vue-router'
import { getPathForPlatform } from '../paths'

const initialized = ref(false)
Expand All @@ -22,43 +21,38 @@ export function useUnifiedRunner () {

return {
initialized: readonly(initialized),

watchSpec: (specs: Ref<ReadonlyArray<SpecFile>>) => {
watchSpecs: (specs: Ref<ReadonlyArray<SpecFile>>) => {
const specStore = useSpecStore()
const route = useRoute()
const selectorPlaygroundStore = useSelectorPlaygroundStore()

watch(() => specs.value, (newVal) => {
const fileParam = getPathForPlatform(route.query.file as string)
watchEffect(() => {
const queryFile = getPathForPlatform(route.query.file as string)

if (!fileParam) {
if (!queryFile) {
// no file param, we are not showing a file
// so no action needed when specs list updates
return
}

const activeSpecInSpecsList = newVal.find((x) => x.relative === fileParam)
const activeSpecInSpecsList = specs.value.find((x) => x.relative === queryFile)

if (!activeSpecInSpecsList) {
// the specs list no longer contains the spec being shown
// so set active state to null and let the UI handle it
if (activeSpecInSpecsList && specStore.activeSpec?.relative !== activeSpecInSpecsList.relative) {
specStore.setActiveSpec(activeSpecInSpecsList)
} else if (!activeSpecInSpecsList) {
specStore.setActiveSpec(null)
}
})

return watch(() => getPathForPlatform(route.query.file as string), (queryParam) => {
const spec = specs.value.find((x) => x.relative === queryParam)

watch(() => getPathForPlatform(route.query.file as string), () => {
if (selectorPlaygroundStore.show) {
const autIframe = getAutIframeModel()

autIframe.toggleSelectorPlayground(false)
selectorPlaygroundStore.setEnabled(false)
selectorPlaygroundStore.setShow(false)
}

specStore.setActiveSpec(spec ?? null)
}, { immediate: true, flush: 'post' })
}, { flush: 'post' })
},
}
}