-
Notifications
You must be signed in to change notification settings - Fork 1
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
Added iframe mocks support #17
base: main
Are you sure you want to change the base?
Conversation
8f121a9
to
30aca48
Compare
@deviantintegral this is WIP, but would love your opinion on the direction. |
}, | ||
"packageManager": "[email protected]+sha512.a6b2f7906b721bba3d67d4aff083df04dad64c399707841b7acf00f6b133b7ac24255f2652fa22ae3534329dc6180534e98d17432037ff6fd140556e2bb3137e" |
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.
Do we need this? We don't have a yarn lockfile or the like.
import {Mockable} from "../../testcase"; | ||
import {Youtube} from "./youtube"; | ||
|
||
export class Mock implements Mockable{ |
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's not clear to me that loading a Mock here actually does something with YouTube. I'm not clear why we need this extra Mock class. Would it be clearer to have YouTubeMock Implements Mockable
?
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.
This is definitely bad naming. This extra Mock class was using in some projects to call all the mocks mock method. As we only have youtube here for now, makes not a lot of sense. Maybe renaming to EveryIframeServiceMock would make sense.
Adds iframe mocks support
TODO:
Usage