Skip to content

Conversation

vshanthe
Copy link
Contributor

📝 Description

Address some of the failing tests

✔️ How to Test

make test-int TEST_CASE=test_nodebalancer.py
make test-int TEST_CASE=test_firewall.py
make test-int TEST_CASE=test_account.py

📷 Preview

If applicable, include a screenshot or code snippet of this change. Otherwise, please remove this section.

@vshanthe vshanthe requested a review from a team as a code owner September 19, 2025 13:30
@vshanthe vshanthe requested review from yec-akamai and zliang-akamai and removed request for a team September 19, 2025 13:30
@vshanthe vshanthe requested a review from Copilot September 22, 2025 05:48
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes failing integration tests by addressing various test reliability and assertion issues. The changes focus on improving test stability, fixing timeouts, and correcting API usage patterns.

  • Removes blank lines in unit test files to clean up formatting
  • Updates integration tests to use proper fixture dependencies and improve error handling
  • Fixes timing-sensitive assertions and test logic to reduce flakiness

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/unit/objects/monitor_test.py Removes blank lines for formatting cleanup
test/unit/objects/linode_test.py Removes blank lines for formatting cleanup
test/integration/models/nodebalancer/test_nodebalancer.py Adds proper fixture dependencies and improves cleanup with error handling
test/integration/models/firewall/test_firewall.py Refactors tests to use existing firewall associations instead of creating new ones
test/integration/models/account/test_account.py Increases timeout threshold and improves event polling logic
test/integration/helpers.py Adds duplicate sleep call in retry logic

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Contributor

@ezilber-akamai ezilber-akamai left a comment

Choose a reason for hiding this comment

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

Nice work!

@zliang-akamai
Copy link
Member

It's still failing... https://github.com/linode/linode_api4-python/actions/runs/18012149846
I think for firewall, you would probably need to create the device before testing on getting it.

@vshanthe
Copy link
Contributor Author

vshanthe commented Sep 29, 2025

It's still failing... https://github.com/linode/linode_api4-python/actions/runs/18012149846 I think for firewall, you would probably need to create the device before testing on getting it.

i have tried that way as well in that case we get - "Too many active firewalls attached to Linode seibylty "
the above test are passing in the local , will try to check if i can change the implementing way

@vshanthe vshanthe requested a review from PawelSnoch October 8, 2025 07: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.

3 participants