fix: scope config to current testing type #20677
Conversation
|
Thanks for taking the time to open a PR!
|
…jandro/fix/scope-config-to-current-testing-type
|
Test summary
Run details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
| @@ -296,12 +296,12 @@ describe('lib/util/config-file-updater', () => { | |||
| } | |||
| ` | |||
|
|
|||
| const output = await insertValueInJSString(src, { component: { specFilePattern: 'src/**/*.spec.cy.js' } }) | |||
| cy.get('.bg-teal-100').contains('tests/**/*') | ||
| cy.get('.bg-teal-100').contains('tests/_support/spec_helper.js') |
There was a problem hiding this comment.
what caused this to change?
There was a problem hiding this comment.
The reason is that now we are not returning the values as a nested property of the e2e or component and bc it was set as from config the whole object was highlighted
| @@ -98,6 +98,11 @@ const isValidConfig = (key: string, config: any) => { | |||
| return true | |||
| } | |||
|
|
|||
| export const defaultSpecPattern = { | |||
There was a problem hiding this comment.
Can we update the e2e & component configuration values to return these?
Nevermind you did. Why do these need to be exported separately? Can't we pull these values from the resolved config?
There was a problem hiding this comment.
It needs to be exported separately because it replaces this file that was removed - also we can't get the values from the resolved config bc there's no gonna be nested properties based on the testing type - just the top-level ones, so we do not now the default value if it's overridden by the one set on the config file and also we may not be able to know the default value of the testing type that is not being selected
| @@ -500,6 +505,12 @@ const runtimeOptions: Array<RuntimeConfigOption> = [ | |||
| validation: validate.isString, | |||
| isInternal: true, | |||
| canUpdateDuringTestTime: false, | |||
| }, { | |||
| name: 'additionalIgnorePattern', | |||
There was a problem hiding this comment.
Looks like a new config value I had missed. Will add to the changelog. Are others able to add to this or is this internal to Cypress? I'd expect the excludeSpecPattern to be leverage instead of needing to add another option as well.
Can we scope this to the component config? it appears to be specific to component testing.
There was a problem hiding this comment.
Looking below, do we need this at all? Seems like we can just pull the specPattern and it'll resolve to either the default e2e specPttern or the custom specPattern provided: https://github.com/cypress-io/cypress/pull/20677/files#diff-0349e7b425446de9ccaf2edbb623f43cffd420688dd312850417888318b9f609R162
There was a problem hiding this comment.
This value is only used internally when we are getting the specs for the CT we want to ignore as well the specPattern that is set on the e2e property, the reason why we can just pull the specPattern is bc know that we are removing the nested properties, we are not gonna be able to know what was the value set on the e2e, the one that is going to be on the top level is the one of the CT - so we may end up ignoring the specPattern that we are looking for
| @@ -217,7 +217,7 @@ export class ProjectDataSource { | |||
|
|
|||
| const MINIMATCH_OPTIONS = { dot: true, matchBase: true } | |||
|
|
|||
| const { specPattern = [], excludeSpecPattern = [] } = await this.ctx.project.specPatternsForTestingType(this.ctx.currentProject, this.ctx.coreData.currentTestingType) | |||
| const { specPattern = [], excludeSpecPattern = [] } = await this.ctx.project.specPatterns() | |||
There was a problem hiding this comment.
If we don't remove the ignoreSpecPattern config option, shouldn't it be taken into account here?
| @@ -203,6 +211,11 @@ export function mergeDefaults ( | |||
| throw errors.get(err, ...args) | |||
| }) | |||
|
|
|||
| delete config['e2e'] | |||
There was a problem hiding this comment.
Can you add a comment on why this is being done? I doesn't feel obvious if I were to read this later.
…jandro/fix/scope-config-to-current-testing-type
…github.com:cypress-io/cypress into alejandro/fix/scope-config-to-current-testing-type
estrada9166
deleted the
alejandro/fix/scope-config-to-current-testing-type
branch
User facing changelog
Additional details
How has the user experience changed?
PR Tasks
cypress-documentation?type definitions?cypress.schema.json?