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

chore: unskip node streaming errors tests #411

Open
1 task
lilnasy opened this issue Mar 4, 2024 · 3 comments
Open
1 task

chore: unskip node streaming errors tests #411

lilnasy opened this issue Mar 4, 2024 · 3 comments
Labels
- P1: chore Doesn't change code behavior (priority)

Comments

@lilnasy
Copy link
Contributor

lilnasy commented Mar 4, 2024

Describe the Bug

  • Since Improve Node.js performance using an AsyncIterable astro#9614, some node tests that check chunk size, and count etc. have been failing.
  • They didn't run with the PR because it did not change any code within the node integration itself - Turborepo rules skipped them.
  • When they finally ran days afterwards, they had to be skipped.

What's the expected result?

The tests get executed.

Link to Minimal Reproducible Example

https://github.com/withastro/astro/blob/main/packages/integrations/node/test/errors.test.js

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Mar 4, 2024
@matthewp matthewp added the - P1: chore Doesn't change code behavior (priority) label Mar 4, 2024
@github-actions github-actions bot removed the needs triage Issue needs to be triaged label Mar 4, 2024
@ematipico
Copy link
Member

We should actually remove turborepo caching when running the tests - always. I don't know turborepo very well though, I don't know how to configure that

@lilnasy
Copy link
Contributor Author

lilnasy commented Mar 4, 2024

I agree.

@bluwy
Copy link
Member

bluwy commented Mar 5, 2024

I don't think we should remove turborepo. The issue is that we did not configure the dependency of Astro to the other integrations, so when Astro core is updated, turborepo didn't invalidate the cache for the other integrations.

The last time I checked, I didn't fix it is because it'll cause way longer test times, but if we're fine with the tradeoff for more robust testing, we can definitely fix that.

If we remove turborepo entirely, tests will unnecessarily run. For example, if we update the @astrojs/vercel only, we should only run its tests and not Astro core.

@ascorbic ascorbic transferred this issue from withastro/astro Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P1: chore Doesn't change code behavior (priority)
Projects
None yet
Development

No branches or pull requests

4 participants