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

bug: Test files copied to output target #5788

Closed
3 tasks done
mrtnmgs opened this issue May 22, 2024 · 9 comments · Fixed by #5789 or ionic-team/ionic-framework#29666
Closed
3 tasks done

bug: Test files copied to output target #5788

mrtnmgs opened this issue May 22, 2024 · 9 comments · Fixed by #5789 or ionic-team/ionic-framework#29666
Labels
Bug: Validated This PR or Issue is verified to be a bug within Stencil

Comments

@mrtnmgs
Copy link

mrtnmgs commented May 22, 2024 •
edited
Loading

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 #5781. I had not realized the problem was much larger than just the mocks.
@ionitron-bot ionitron-bot bot added the triage label May 22, 2024
@alicewriteswrongs
Copy link
Contributor

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 mrtnmgs mentioned this issue May 22, 2024
Closed
3 tasks
@mrtnmgs
Copy link
Author

mrtnmgs commented May 22, 2024 •
edited
Loading

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
Copy link
Contributor

Agree, this is a bug 👍

@alicewriteswrongs alicewriteswrongs added the Bug: Validated This PR or Issue is verified to be a bug within Stencil label May 22, 2024
@ionitron-bot ionitron-bot bot removed the triage label May 22, 2024
alicewriteswrongs added a commit that referenced this issue May 22, 2024
@alicewriteswrongs
fix(emit): don't emit test files
29fcf57 
This fixes a bug introduced in #4315, which upgraded to TypeScript 5. In
that PR we had to change the way that we prevented certain files from
being emitted by the typescript compiler because in the 5.0 release our
previous approach stopped working.

However, in porting over to a new approach that worked with TS 5.0 there
was an oversight. I misunderstood the intent of the old code as being to
merely prevent writing `.d.ts` files in the output, when actually it was
about preventing the compiled JavaScript from being written for
`.e2e.ts` and `.spec.ts` files.

This change ensures that we no longer emit these files.

fixes #5788
STENCIL-1325
@alicewriteswrongs alicewriteswrongs mentioned this issue May 22, 2024
Merged
2 tasks
@alicewriteswrongs
Copy link
Contributor

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
Copy link
Author

mrtnmgs commented May 23, 2024

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
Copy link
Contributor

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 added a commit that referenced this issue Jun 3, 2024
@alicewriteswrongs
fix(emit): don't emit test files
9426115 
This fixes a bug introduced in #4315, which upgraded to TypeScript 5. In
that PR we had to change the way that we prevented certain files from
being emitted by the typescript compiler because in the 5.0 release our
previous approach stopped working.

However, in porting over to a new approach that worked with TS 5.0 there
was an oversight. I misunderstood the intent of the old code as being to
merely prevent writing `.d.ts` files in the output, when actually it was
about preventing the compiled JavaScript from being written for
`.e2e.ts` and `.spec.ts` files.

This change ensures that we no longer emit these files.

fixes #5788
STENCIL-1325
@alicewriteswrongs
Copy link
Contributor

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.

alicewriteswrongs added a commit that referenced this issue Jun 3, 2024
@alicewriteswrongs
fix(emit): don't emit test files
715f36c 
This fixes a bug introduced in #4315, which upgraded to TypeScript 5. In
that PR we had to change the way that we prevented certain files from
being emitted by the typescript compiler because in the 5.0 release our
previous approach stopped working.

However, in porting over to a new approach that worked with TS 5.0 there
was an oversight. I misunderstood the intent of the old code as being to
merely prevent writing `.d.ts` files in the output, when actually it was
about preventing the compiled JavaScript from being written for
`.e2e.ts` and `.spec.ts` files.

This change ensures that we no longer emit these files.

fixes #5788
STENCIL-1325
@mrtnmgs
Copy link
Author

mrtnmgs commented Jun 4, 2024

Thanks for the update @alicewriteswrongs !

github-merge-queue bot pushed a commit that referenced this issue Jun 4, 2024
@alicewriteswrongs
fix(emit): don't emit test files (#5789)
50892f1 
This fixes a bug introduced in #4315, which upgraded to TypeScript 5. In
that PR we had to change the way that we prevented certain files from
being emitted by the typescript compiler because in the 5.0 release our
previous approach stopped working.

However, in porting over to a new approach that worked with TS 5.0 there
was an oversight. I misunderstood the intent of the old code as being to
merely prevent writing `.d.ts` files in the output, when actually it was
about preventing the compiled JavaScript from being written for
`.e2e.ts` and `.spec.ts` files.

This change ensures that we no longer emit these files.

fixes #5788
STENCIL-1325
@tanner-reits
Copy link
Contributor

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

@brandyscarney brandyscarney mentioned this issue Jun 26, 2024
Merged
github-merge-queue bot pushed a commit to ionic-team/ionic-framework that referenced this issue Jun 26, 2024
@brandyscarney
chore(deps): update dependency @stencil/core to v4.19.0 (#29666)
52ff050 
### Release Notes

<details>
<summary>ionic-team/stencil (@&#8203;stencil/core)</summary>

###
[`v4.19.0`](https://togithub.com/ionic-team/stencil/blob/HEAD/CHANGELOG.md#-4190-2024-06-26)

[Compare
Source](https://togithub.com/ionic-team/stencil/compare/v4.18.3...v4.19.0)

### Bug Fixes

* **compiler:** support rollup's external input option
([#3227](stenciljs/core#3227))
([2c68849](stenciljs/core@2c68849)),
fixes [#3226](stenciljs/core#3226)
* **emit:** don't emit test files
([#5789](stenciljs/core#5789))
([50892f1](stenciljs/core@50892f1)),
fixes [#5788](stenciljs/core#5788)
* **hyrdate:** support vdom annotation in nested dsd structures
([#5856](stenciljs/core#5856))
([61bb5e3](stenciljs/core@61bb5e3))
* label attribute not toggling input
([#3474](stenciljs/core#3474))
([13db920](stenciljs/core@13db920)),
fixes [#3473](stenciljs/core#3473)
* **mock-doc:** expose ShadowRoot and DocumentFragment globals
([#5827](stenciljs/core#5827))
([98bbd7c](stenciljs/core@98bbd7c)),
fixes [#3260](stenciljs/core#3260)
* **runtime:** allow watchers to fire w/ no Stencil members
([#5855](stenciljs/core#5855))
([850ad4f](stenciljs/core@850ad4f)),
fixes [#5854](stenciljs/core#5854)
* **runtime:** catch errors in async lifecycle methods
([#5826](stenciljs/core#5826))
([87e5b33](stenciljs/core@87e5b33)),
fixes [#5824](stenciljs/core#5824)
* **runtime:** don't register listener before connected to DOM
([#5844](stenciljs/core#5844))
([9d7021f](stenciljs/core@9d7021f)),
fixes [#4067](stenciljs/core#4067)
* **runtime:** properly assign style declarations
([#5838](stenciljs/core#5838))
([5c10ebf](stenciljs/core@5c10ebf))
* **testing:** allow to re-use pages across it blocks
([#5830](stenciljs/core#5830))
([561eab4](stenciljs/core@561eab4)),
fixes [#3720](stenciljs/core#3720)
* **typescript:** remove unsupported label property
([#5840](stenciljs/core#5840))
([d26ea2b](stenciljs/core@d26ea2b)),
fixes [#3473](stenciljs/core#3473)


### Features

* **cli:** support generation of sass and less files
([#5857](stenciljs/core#5857))
([1883812](stenciljs/core@1883812)),
closes [#2155](stenciljs/core#2155)
* **compiler:** generate export maps on build
([#5809](stenciljs/core#5809))
([b6d2404](stenciljs/core@b6d2404))
* **complier:** support type import aliasing
([#5836](stenciljs/core#5836))
([7ffb25d](stenciljs/core@7ffb25d)),
closes [#2335](stenciljs/core#2335)
* **runtime:** support declarative shadow DOM
([#5792](stenciljs/core#5792))
([c837063](stenciljs/core@c837063)),
closes [#4010](stenciljs/core#4010)
* **testing:** add `toHaveLastReceivedEventDetail` event spy matcher
([#5829](stenciljs/core#5829))
([63491de](stenciljs/core@63491de)),
closes [#2488](stenciljs/core#2488)
* **testing:** allow to disable network error logging via
'logFailingNetworkRequests' option
([#5839](stenciljs/core#5839))
([dac3e33](stenciljs/core@dac3e33)),
closes [#2572](stenciljs/core#2572)
* **testing:** expose captureBeyondViewport in pageCompareScreenshot
([#5828](stenciljs/core#5828))
([cf6a450](stenciljs/core@cf6a450)),
closes [#3188](stenciljs/core#3188)

</details>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug: Validated This PR or Issue is verified to be a bug within Stencil
Projects
None yet
Milestone
No milestone
3 participants
@alicewriteswrongs @mrtnmgs @tanner-reits