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.

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: add new sample mp3s for e2e tests and storybook #927

Merged
merged 3 commits into from
Jan 23, 2024

Conversation

lhaggar
Copy link
Member

@lhaggar lhaggar commented Jan 18, 2024

What
Previous mp3 test files used in Storybook and E2E tests has been removed, meaning E2E tests fail. This PR updates references to a new file, which will be self-hosted under the newskit.co.uk website. This PR will require forced merging with broken E2E and deploying through to prod. Subsequent builds should then work as normal.

I have done:

  • Written unit tests against changes
  • Written functional tests against the component and/or NewsKit site
  • Updated relevant documentation

I have tested manually:

  • The feature's functionality is working as expected on Chrome, Firefox, Safari and Edge
  • The screen reader reads and flows through the elements as expected.
  • There are no new errors in the browser console coming from this PR.
  • When visual test is not added, it renders correctly on different browsers and mobile viewports (Safari, Firefox, small mobile viewport, tablet)
  • The Playground feature is working as expected

@github-actions github-actions bot added the fix This change fixes a bug label Jan 18, 2024
@pp-serviceaccount
Copy link
Collaborator

@lhaggar lhaggar marked this pull request as ready for review January 18, 2024 16:10
@lhaggar lhaggar requested a review from a team as a code owner January 18, 2024 16:10
JohnTParsons
JohnTParsons previously approved these changes Jan 18, 2024
pdimova
pdimova previously approved these changes Jan 19, 2024
Copy link
Contributor

@pdimova pdimova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering what the reason is for removing these files on S3

@lhaggar
Copy link
Member Author

lhaggar commented Jan 19, 2024

@pdimova not 100% sure. The bucket exists but is empty. It's in the NGN dev account rather than NewsKit's, I think its the original bucket before the site origin was moved.

@kboeff kboeff dismissed stale reviews from pdimova and JohnTParsons via 999f825 January 19, 2024 11:08
@JohnTParsons
Copy link
Collaborator

@pdimova not 100% sure. The bucket exists but is empty. It's in the NGN dev account rather than NewsKit's, I think its the original bucket before the site origin was moved.

I thought I'd moved the ncu-newskit-docs bucket from NGN to product platforms account a year ago. Not sure.

I thought NGN was just being used to save the Terraform state.

@lhaggar lhaggar merged commit 13c544a into main Jan 23, 2024
30 of 34 checks passed
@lhaggar lhaggar deleted the fix/new-sample-mp3s-for-e2e branch January 23, 2024 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix This change fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants