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

Add E2E scenario for travel advice publisher #689

Closed
wants to merge 1 commit into from

Conversation

benthorner
Copy link
Contributor

@benthorner benthorner commented Jun 29, 2020

https://trello.com/c/TUB9OGWD/152-prepare-and-run-a-beta-trial-of-continuous-deployment

This is a mergeable demo of how we could extend the coverage provided by
Smokey tests to give us more confidence in changes to publishing apps.

We want to avoid running this in production, as it would generate
changes to the live site. Although each run of this scenario can
generate data, the daily wipes of our non-production DBs should be
sufficient to avoid any longterm build up.

We want to avoid running this in production, as it would generate
changes to the live site. Although each run of this scenario can
generate data, the daily wipes of our non-production DBs should be
sufficient to avoid any longterm build up.
@benthorner
Copy link
Contributor Author

Tested and works on Integration 🎉

@kevindew
Copy link
Member

Thanks for raising - great to get the Smokey tests in hear for apps that aren't covered

I think this test actually goes further than it should and breaks from the convention that Smokey is used for tests that are idempotent. This then throws this suite into contention of purpose with the publishing-e2e-tests, where there is already a test of this nature

I don't think this should go to the level of actually doing a non-idempotent action and should do something that can be done in all environments (I think just a sign-in and see the publisher should be fine, that would have been enough on the big certificate outage of the other week for instance).

It's also a bit of a mine-field getting publishing tests to work reliably due to our publishing working on an eventual consistency model. This test here would pass even if subsequent publishings failed since the text would still be on the document. With the e2e tests there is a rather grim loop approach where we have to reload the page over and over to see if it's updated with the copy (the built in wait for aspect of Capistrano just waits to see if the pages rather than reload it). It can be a pretty slow process and could well drag the time of these tests down.

@benthorner
Copy link
Contributor Author

Thanks for explaining @kevindew ! I think the takeaway for me from this is that we should also promote more reliance on the Publishing E2E tests in the rollout of Continuous Deployment. Giving more attention to both them and Smokey can only be a good thing, and I agree we don't need to duplicate those tests over here.

On a side note: did we ever think about adding E2E tests for Content Publisher?

@benthorner benthorner closed this Jun 29, 2020
@kevindew
Copy link
Member

Thanks for explaining @kevindew ! I think the takeaway for me from this is that we should also promote more reliance on the Publishing E2E tests in the rollout of Continuous Deployment. Giving more attention to both them and Smokey can only be a good thing, and I agree we don't need to duplicate those tests over here.

Sure, I think we should still have some tests for publishing apps into Smokey just that they should be pretty basic and quick ones. They'd also satisfy the question that the application is operable in different environments past beyond the assertion that e2e tests make that the release of the app can function accordingly.

On a side note: did we ever think about adding E2E tests for Content Publisher?

We deferred it initially and then perpetually. Basic logic being that it's so rare that there is an API issue with other apps that the process adds far more pain than value.

We may want to rethink them in the age of CD. I recall our eventual switch to CD being one of the big arguments towards keeping them. They could really use some iteration though, they suffer a lot from having a single set-up step that needs to build all the apps before running tests for just the apps in question. May be worth setting something govuk-dev-tools up around them and Smokey to consider both projects futures?

@barrucadu barrucadu deleted the publish-e2e-test branch June 11, 2021 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants