-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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: add baseUrl to TestConfigOverrides #22445
Conversation
Thanks for taking the time to open a PR!
|
Do we still get a warning if we do: import { defineConfig } from 'cypress'
export default defineConfig({
baseUrl: 'http://...'
}) This should be invalid. The valid way would be: import { defineConfig } from 'cypress'
export default defineConfig({
e2e: {
baseUrl: 'http://...'
}
}) It looks like this should still work as expected, just looking to clarify. Also, this might fix #22072. If it's not too much extra work, could you see if this issue is fixed by your change? (and ideally add a test, if we haven't got one). This should work and be correctly typed, now, too: it('something', { baseUrl: '...' }, () => {
// ...
}) We have no way to know at compile time if the current test is |
@imiller thanks for checking!!
👇 This still warns as expected when trying to use
There are existing Edit: fixed the links to the tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good and does close #22072 as well. I added that to the description. Thanks for the PR!
@imadx I will get this one merged up, thanks! |
I re-pushed the same commit and created a new CI run, and the failing tests are fine: https://github.com/cypress-io/cypress/compare/issue-22445-rerun The fails here are some unrelated. I'm confident they are not related to this change, I am going to merge this. |
I am still finding out the problem with CI. Please wait a bit - I'm going to get this merged as soon as I find out why your branch fails, but my identical fork doesn't. |
@lmiller1990, is it okay if I rebase from Or if you have a passing fork, I think it's okay if we can get that merged if someone is waiting on this. |
I actually have the rebased changes here at #22532 |
Thanks, I merged in develop that should have a fix that might help with the CI. Specifically this PR landed #22326. Let's see how it goes. |
Test summaryRun 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 |
We did it, it's passing. Let's merge. |
Nice one. Thanks, @lmiller1990 for getting this through 🚀 |
Co-authored-by: Lachlan Miller <[email protected]>
* fix: distribute files to machines for external contributors. (#22326) * fix: distribute files to machines for external contributors. * fix path * fix * fix glob * fix * fix glob pattern spec->cy. * fix * echo things. * test * use cd. * fix component tests. * test * test * fix * refactor * test distribut-step fix error fix fix test TEST * Revert "test distribut-step" This reverts commit 15c3606. * Revert "refactor" This reverts commit 21a8ad9. * reduce flake by increasing viewport height Co-authored-by: Ryan Manuel <[email protected]> Co-authored-by: Lachlan Miller <[email protected]> * fix: add baseUrl to TestConfigOverrides (#22445) Co-authored-by: Lachlan Miller <[email protected]> * handle white space when registering ts-node using --require * update test project * move config Co-authored-by: Kukhyeon Heo <[email protected]> Co-authored-by: Ryan Manuel <[email protected]> Co-authored-by: Ishan Madhusanka <[email protected]>
It should be out now in 10.3.0! |
TestConfigOverrides
#22072User-facing changelog
Corrected the Typescript types to include
baseUrl
as a valid test config override option. Fixes #22374 and #22072.Additional details
The documentation mentions that
baseUrl
is also configurable and can be changed at run time.https://docs.cypress.io/guides/references/configuration#Test-Configuration
However, the types are missing
baseUrl
there forTestConfigOverrides
.Steps to test
Updating
baseUrl
in a test withCypress.config("baseUrl", baseUrl);
can be used to test the behaviour.How has the user experience changed?
Before
After
There is no typescript warning
PR Tasks
cypress-documentation
?type definitions
?