Conversation
- Replaced unittest.TestCase with pytest fixtures and native assertions - Implemented test/conftest.py with shared adyen_instance and mock_client fixtures - Renamed all test files to follow snake_case convention (test_*.py) - Removed obsolete test/BaseTest.py - Updated tox.ini and Makefile to use pytest as default test runner - Fixed brittle HMAC tests and incorrect API method calls identified during migration
- Implemented session-scoped mock_server fixture in conftest.py - Added dynamic mock file serving capability via X-Mock-File header - Created test/test_http_backends.py to verify real network communication - Validated all supported backends (requests, pycurl, urllib) against the wire - Added timeout resiliency tests for each networking stack
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| except: | ||
| response["received_body"] = post_data.decode('utf-8') | ||
|
|
||
| self.wfile.write(json.dumps(response).encode('utf-8')) |
Check failure
Code scanning / SonarCloud
Endpoints should not be vulnerable to reflected cross-site scripting (XSS) attacks High test
| return None | ||
|
|
||
| # Try to find the file | ||
| if not os.path.exists(filename): |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High test
| self.wfile.write(f"Mock file not found: {filename}".encode()) | ||
| return True | ||
|
|
||
| with open(filename, 'rb') as f: |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High test
Summary of ChangesHello @thomasc-adyen, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly upgrades the project's testing capabilities by transitioning from the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a significant and well-executed migration of the test suite from unittest to pytest. The introduction of pytest features like fixtures (in conftest.py) and parameterized tests modernizes the testing infrastructure and improves maintainability. The new integration tests using a localhost server are a great addition for verifying the behavior of different HTTP backends.
I've found a couple of areas for improvement in the new test files, mainly related to test structure and efficiency. My comments are detailed below. Overall, this is an excellent improvement to the project's testing capabilities.
| """Verifies that timeouts are handled (using a non-responsive endpoint if possible).""" | ||
| # This is a bit harder with http.server as it is single-threaded and handles one request at a time. | ||
| # But we can verify that the timeout parameter is at least passed correctly. | ||
| ady = Adyen.Adyen() | ||
| ady.client.http_timeout = 0.001 | ||
| ady.client.http_force = "requests" | ||
| ady.client._init_http_client() | ||
|
|
||
| # We expect a timeout error from the backend | ||
| with pytest.raises(Exception): | ||
| ady.client.http_client.request( | ||
| "GET", | ||
| f"{mock_server}/slow-endpoint", | ||
| xapikey="TEST_XAPI_KEY" | ||
| ) |
There was a problem hiding this comment.
This timeout test logic appears to be misplaced within the test_backend_with_real_mock_file function and will run as part of it. It should be extracted into its own separate test function to ensure proper test isolation and clarity. Additionally, the test is hardcoded for the 'requests' backend. It would be more robust to parameterize it to cover all supported HTTP backends ('requests', 'pycurl', 'urllib'), similar to test_backend_communication.
| with open(filename) as data_file: | ||
| data = json.load(data_file) | ||
| with open(filename) as st: | ||
| strjson = st.read() |
There was a problem hiding this comment.
This implementation opens and reads the same file twice, which is inefficient. The file content can be read once into a string, and then parsed as JSON. This avoids redundant I/O operations.
with open(filename, 'r') as data_file:
strjson = data_file.read()
data = json.loads(strjson)1 participant
Description
Migrates the project to use
pytestand a more modernized testing infrastructure, and introduces integration tests using a localhost server that loads data from mock files.