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.

Sign up for GitHub

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

Jump to bottom

Enable BASE_URL tests #1828

Merged
merged 1 commit into from May 6, 2024
Merged

Enable BASE_URL tests #1828

merged 1 commit into from May 6, 2024

Conversation

HiDeoo
Copy link
Member

@HiDeoo HiDeoo commented May 3, 2024

Description

Following the merge of #1613, this PR enables the BASE_URL tests we had in the codebase using a similar approach as the one used to stub the MODE to production in the tests.

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented May 3, 2024

⚠️ No Changeset found

Latest commit: b2b5595

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel Vercel
Copy link

vercel bot commented May 3, 2024 •
edited

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
starlight ✅ Ready (Inspect) Visit Preview May 3, 2024 3:45pm

@github-actions github-actions bot added the 🌟 core Changes to Starlight’s main package label May 3, 2024
HiDeoo
HiDeoo commented May 3, 2024
View reviewed changes
packages/starlight/__tests__/basics/slugs.test.ts
// `vite-plugin-env` controlling it and the lack of a way to pass in an Astro config using
// `getViteConfig()` from `astro/config`.
test.todo('returns slugs with a custom `base` option', () => {
test('returns slugs with a custom `base` option', () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that this test does not need the whole resetModules() → stubEnv() → import() dance as contrary to base.ts, BASE_URL is not used/evaluated at the top-level scope of the module but in the scope of the function under test.

delucis
delucis approved these changes May 3, 2024
View reviewed changes
Copy link
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

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

Thanks for remembering we had these stubbed out and realising these tests could now work! Lovely work @HiDeoo 💖

@HiDeoo HiDeoo merged commit 24b92ca into withastro:main May 6, 2024
12 checks passed
@HiDeoo HiDeoo deleted the hd-base-tests branch May 6, 2024 08:13
HiDeoo added a commit to HiDeoo/starlight that referenced this pull request May 8, 2024
@HiDeoo
Merge branch 'main' into hd-refactor-astro-i18n
c193d55 
* main:
  i18n(zh-cn): Update docs about synced-tabs (withastro#1834)
  i18n(zh-cn): Update some docs about withastro#1620 & withastro#1613 (withastro#1835)
  Add more diagnostic help to error messages thrown by `<Steps>` (withastro#1838)
  i18n(zh-cn): Update components.mdx (withastro#1836)
  i18n(zh-cn): Update community-content.mdx (withastro#1833)
  Improve type checking job (withastro#1831)
  [ci] format
  [ci] release (withastro#1832)
  i18n(ru): update ru.json (withastro#1826)
  Fix `<Tabs>` sync issue with inconsistent use of `icon` on `<TabItem>` components (withastro#1811)
  Enable `BASE_URL` tests (withastro#1828)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@delucis delucis delucis approved these changes

Assignees
No one assigned
Labels
🌟 core Changes to Starlight’s main package
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@HiDeoo @delucis