-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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: Intregrate runner packages. #23028
Merged
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
54b73b5
chore: Intregrate runner packages.
sainthkh e836b14
Remove unnecessary Studio react files.
sainthkh 1c4e85e
Remove unnecessary gif
sainthkh 73fdfb4
runner-shared to runner-ct.
sainthkh 8fab511
fix path.
sainthkh a00e2aa
fix package.json
sainthkh e636e95
Remove scss files from runner-ct
sainthkh d1c7754
Remove runner-ct
sainthkh e1b887f
Remove runner-shared and runner-ct comments.
sainthkh 166cc8a
Feedback
sainthkh 73fc73c
Merge branch 'develop' into issue-21689
emilyrohrbough a4b4f7f
chore: reduce parallelism for reporter-componen-tests
lmiller1990 cccd76e
chore: reduce paralelleism
lmiller1990 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Validating CODEOWNERS rules …
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file was deleted.
Oops, something went wrong.
This file was deleted.
Oops, something went wrong.
This file was deleted.
Oops, something went wrong.
36 changes: 0 additions & 36 deletions
36
packages/runner-ct/src/SpecList/components/SearchSpec.scss
This file was deleted.
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI some context - we ideally should delete ALL of runner/runner-shared, and bundle the entire app with the Vite stack.
The reason we are stuck like this is explained a little here: #20663 (comment)
Basically some of the code in driver and possibly reporter are not really ESM compatible - webpack does something different to Vite to resolve the modules, which is why it works in webpack, but we cannot import to Vite.
Not sure if this is something you are looking to investigate more @sainthkh but if you do please let me know, I'd like to follow along/help out - I'd really like to get away from this "inject webpack bundle" mess at some point, but it's quite hard. Likely making driver 100% ESM (including dependencies) would be a good first step.
Any thoughts on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that we have 4 things to do to delete the
runner
pacakge.dom
. -> I thinkdriver/src/dom
might be a good place or move it toapp
directly.driver
-> I agree it's the biggest problem.reporter
compile its own styles. -> Moverunner/webpack.config.ts
toreporter
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dom -> are you going do to this here?
Studio -> I am looking into this now
ESMify driver -> agree, hard
Reporter styles -> do this separately I think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I read the
dom
code and it seems that it's better to be a separate PR.It currently renders highlight by
I can simply migrate
dom
toapp
and useunified-runner
. But I don't think it's not our goal. It's just hiding and postponing.To get migrating
dom
done, I need to do these things:dom
anddimensions
.dimensions
todriver/src/dom
because it's used indriver/src/dom/blackout
.Highlight
fromreact
tovue
with ourTooltip
component.dom
functions toapp/runner/aut-iframe
. Leavestudio
-related functions.dom
andCypressJQuery
. They're only used insideaut-iframe
. I guess they can be removed safely fromunified-runner
.I think it's a big change and outside the scope of this PR.
As for reporter styles, it's safe to do it after ESM-ifying
driver
. We're trying to removewebpack
. I don't think we don't need to create one morewebpack.config.ts
now.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I opened the new PR #23187 to migrate
dom.js
. It's currently empty because I'm now reading and planning.