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

added e2e tests for CORS reddit api #330

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

abadoni5
Copy link
Contributor

@abadoni5 abadoni5 commented Mar 12, 2024

Summary

closes #213, the original lessons used CORS, which failed the tests, since in pr #320 we are using new api (api.reddit.com) I have fixed it.

Checklist

  • Variables, functions and comments are translated to Spanish
  • Functions follow underscore notation
  • Spell check done & typos fixed
  • All python code is PEP8 compliant
  • Test coverage with Playwright implemented; locators are Pyhton code
  • Reviewers assigned (all peers & at least 1 mentor)

Screenshots

Screenshot 2024-03-15 at 12 59 58 AM

@abadoni5
Copy link
Contributor Author

@reingart @NicolasSandoval, the previous attempt at adding e2e tests did not explicitly check for errors, I check the status code to check if the api is working properly handling this error earlier.

Copy link

@@ -18,7 +29,6 @@ def test_l45_1(page):
# Test the src attribute from image matches the Facebook URL
assert img_src == "https://graph.facebook.com/ACDC/picture?type=large"

Copy link

@diivi diivi Mar 13, 2024

Choose a reason for hiding this comment

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

Add a newline here to match with the convention (2 newlines after each test).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing out, fixed!

@sujith-rek
Copy link
Collaborator

image
@abadoni5 The test you wrote for this gives this error, due to which test run is failing

@sujith-rek
Copy link
Collaborator

once check and do necessary changes

def test_l45_2():
"""Tests fetching data from the provided URL"""
# Send a GET request to the URL
url = "https://cors.bridged.cc/http://www.reddit.com/r/Python/.json"
Copy link
Collaborator

Choose a reason for hiding this comment

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

i think the tests are failing due to cors in my opinion.
as #320 use same reddit API link.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks Sujith, updated. This will save time.

@abadoni5
Copy link
Contributor Author

abadoni5 commented Mar 14, 2024

@NicolasSandoval, the CORS error now is fixed with required tests keeping in mind the suggestions by @sujith-rek. Due the way GitHub handles running the tests the api is returning a 403 error but the code is running fine locally and the test is working as expected.

The test is working as expected and only fails due to how github handles their testing workflow.

Copy link

Copy link

@NicolasSandoval
Copy link
Member

@abadoni5 Can you try with another "User-Agent". Try this instead:
"Chrome/122.0.0.0"

Copy link

@abadoni5
Copy link
Contributor Author

@NicolasSandoval added the new header, tests on github still fail. The test still works as expected locally. Let me know if you want any changes.

Copy link

Copy link

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.

Reddit sample broken [TWP45]
4 participants