-
Notifications
You must be signed in to change notification settings - Fork 0
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
fix: Promo Video updates #594
Conversation
Showing how pre-existing non-video Promos haven't been affected: https://deploy-preview-594--cr-component-library.netlify.app/#!/Promo/1 |
Sorry Mike, found a few issues; will reassign for review once it's actual sorted :/ |
@KrupaPammi, any idea what's happening with Playwright here? I wasn't aware we had any PW testing in Component Library, plus I can't actually see any PW processes running under Actions so this just seems to be blocking my work :/ |
Hi @AndyEPhipps, it's part of this PR here that I had to park to look into FSU and Donate tests failing issues. I had to add playwright workflow to run on Github actions but will disable it now. So, the change of cypress to playwright is due to intermittent cypress test failures and some outdated tests. What I thought was to add playwright tests which are fast, and add only functionality-related tests instead of every single component, as they are already covered in Jest tests. Please let me know if this is of any concern. |
Ah right okay, thank you for the clarification; that all sounds sensible to me. What do you think would be the timeline for that work, do you have much more to do in that PR? Would it just be possible to remove the branch protection rule for now without it disrupting your work? I guess in the worst case, we just have to force-merge these PRs while you're working on the PW stuff.. I'm not sure if that'll stop the NPM Publish job happening here or not, but I'll give it a go now! |
@AndyEPhipps I have now removed the PW workflow from GitHub Actions. |
Thanks @KrupaPammi, but do let me know if this just makes your life more difficult in terms of running and confirming the tests you're developing actually outside of your local? |
@AndyEPhipps I can work on those PW tests locally, as I now know the setup on GitHub actions is working fine so keeping the PW workflow on branch rules disabled should be fine until the tests are ready to be reviewed and merged. By this, none of the PRs or merging will be affected. |
Okay brilliant, thank you! |
🎉 This PR is included in version 7.22.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
PR description
What is it doing?
Some additions and simplification to make the new functionality better fit for purpose
Why is this required?
Good content = nice
link to Jira ticket:
No ticket, just an extension of the original rly
Quick Checklist:
My PR title follows the Conventional Commit spec.
I have filled out the PR description as per the template above.
I have added tests to cover new or changed behaviour.to follow in a separate PRI have updated any relevant documentation.
Important! - lastly, make sure to squash merge...