ionic-team / stencil

A toolchain for building scalable, enterprise-ready component systems on top of TypeScript and Web Component standards. Stencil components can be distributed natively to React, Angular, Vue, and traditional web developers from a single, framework-agnostic codebase.
https://stenciljs.com
Other
12.52k stars 782 forks source link

bug: Test files copied to output target #5788

Closed mrtnmgs closed 3 months ago

mrtnmgs commented 4 months ago

Prerequisites

Stencil Version

4.18.0

Current Behavior

In a new stencil project, test files are copied over to dist. In some cases, Jest attempts to run these files, creating errors

Expected Behavior

Test files should be excluded of the distribution files (or, if there is a reason to copy them there, be ignored by jest)

System Info

Tested on both Windows and MacOS, npm 10.7.0

Steps to Reproduce

npm test find dist -type f -name '*.spec.*' find dist -type f -name '*.e2e.*'

Code Reproduction URL

https://github.com/mrtnmgs/mockbug

Additional Information

This is similar to the bug I logged a couple days ago https://github.com/ionic-team/stencil/issues/5781. I had not realized the problem was much larger than just the mocks.

alicewriteswrongs commented 4 months ago

Hey @mrtnmgs this is the same issue as #5781, we'd be resolving it with the same change either way. If you wouldn't mind, could you close whichever one of the two you is less general? It seems like this one is more general to me, so we might close #5781 in favor of this one, but up to you.

mrtnmgs commented 4 months ago

Just closed the other one. I would appreciate if this was considered a bug and not a feature request like the other ticket. The output of npm test is 14000 lines long of false negatives, I basically can't use the test suite. This issue also prevents Continuous Integration.

alicewriteswrongs commented 4 months ago

Agree, this is a bug 👍

alicewriteswrongs commented 4 months ago

Hey @mrtnmgs I just put together a fix which prevents Stencil test files (.spec.ts) from being written to the output - if you have time you could give it a try to see if it fixes the issue for you!

npm install @stencil/core@4.18.2-dev.1716402339.29fcf57
mrtnmgs commented 4 months ago

Thanks @alicewriteswrongs. It works. The patch prevents both .spec.ts and .e2e.ts files to be copied over, and the test suite runs fine. It's still copying the mock files and throwing the warning for them though.

alicewriteswrongs commented 4 months ago

Awesome, glad that it works to prevent those from being written! I'm going to confer with the rest of the team today to come up with a good direction on what to do about those __mock__ files, I'll let you know what we come up with!

alicewriteswrongs commented 4 months ago

So after a bit of investigation I determined that the __mock__ issue is a separate issue from this issue where .spec. and .e2e. files are being written to the output. This issue is a regression introduced in Stencil v3.3.0 which is addressed by #5789.

The separate issue with copying the contents of __mocks__ is something which I can reproduce in all versions of Stencil I've tested -- it has to do with Stencil copying .js files in the src/ directory to the dist output.

Sorry about the back-and-forth on this! This issue (#5788) will be addressed by #5789 and I'm going to re-open #5781 to cover that separate issue.

mrtnmgs commented 3 months ago

Thanks for the update @alicewriteswrongs !

tanner-reits commented 3 months ago

A fix for this was included in today's v4.19.0 release!