-
Notifications
You must be signed in to change notification settings - Fork 50
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
E2E Test Expansion #475
base: develop
Are you sure you want to change the base?
E2E Test Expansion #475
Conversation
Feels asthough we don't need to iterate over EVERY episode, but only 1 seems almost too restrictive and could make dangerous assumptions (i.e. if $latest_episode is OK, they're all OK!). Suggestion: testing the latest 5 episodes and the oldest 5 episodes?
Maybe this is a helpful starting point? #378 (comment) |
Do you think 5 newest across all shows or per show? Also I would advocate for the possibility of picking a specific set of episodes that we should test because not all episodes have guests and I want make sure the Guests section of the episode page is check every time when E2E is run. |
great points @CGBassPlayer !
|
I think that LUP 434 is a great candidate and one I use when working on the episode page layout |
test/e2e/test_404.py
Outdated
""" | ||
@fixture(autouse=True) | ||
def setup(page: Page): | ||
page.goto("/404.html") |
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.
So, for testing the 404 page...should we navigate to it or navigate to a non-existent page and check attributes on that? That way we can test the actual functionality to make sure it's working properly... I don''t know if it really matters either way, but just thinking out loud 🙃
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 say test a random (optionally clever or humorous) URL of your choosing!
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.
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.
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.
Ah....so it looks like it was an improvement between 101 -> 105 when the hugo serve
command received 404 support.
https://github.com/gohugoio/hugo/releases/tag/v0.103.0
Which is why it was working for me, because hugo just updates in the background with the snap package. 😅
I'll do another PR to upgrade the main prod container's version + dev, then we'll be able to properly do 404 testing locally.
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.
Once that PR is merged, I will update to a random url (maybe randomly generated? Or give me some ideas for fun urls!). In the meantime I will keep it as 404.html
Also, I've listed out a bunch of other things/info here: #286 (comment) You've got most of it with your initial comment for this PR, but just figured I'd toss it in to help with some of the things like mocking the request. |
Also, here are some episodes that were particularly tricky from the scrapers perspective. We might want to include those as well: |
So I managed to find a way to build the urls for the episodes we want. I grabs the 5 oldest, and 5 newest episodes per show as well as a whatever we populate in a dictionary in the file. I will look through to see what other edge cases in the issues @elreydetoda pointed out. But the part I think would be the hardest is over. Testing time locally took like 7 minutes through the first run, looking at GH Actions it seems to have taken only 30 seconds. |
BTW @CGBassPlayer, once this stuff gets merged: ChanceM#1 We'll have the live page indicator + mocking (mainly just intercepting the live request and responding with an event and not actual mocking) taken care of. |
a0877ed
to
671c8d6
Compare
Not sure why they PR closed when I pulled the latest changes in... My best guess would be because I rebased my changes on top and so the commit history for my branch has changed? No totally sure. But seems like it good again |
e81f00e
to
812230d
Compare
Adds more page tests to website
(1 episode each since this is templated?)(5 newest, 5 oldest, and any other predefined episodes we want to test)(Should we iterate over all shows or just pick 1 since it is templated?)(All Shows)ScreenshotMoved to playwright handling automatically