chore: unskip node streaming errors tests

withastro / adapters

Home for Astro's core maintained adapters

70 stars 41 forks source link

chore: unskip node streaming errors tests #411

Open lilnasy opened 8 months ago

lilnasy commented 8 months ago

Describe the Bug

Since https://github.com/withastro/astro/pull/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.

ematipico commented 8 months ago

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 commented 8 months ago

I agree.

bluwy commented 8 months ago

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.

ematipico commented 4 weeks ago

No one suggested removing turborepo, but simply removing the caching of the tests. IMHO test tasks shouldn't be cached.