Skip Pubchem live API test in CI#1288
Conversation
Even with many retries and long delays, the test fails for almost every PR. I don't know why; perhaps Pubchem's servers have become overloaded lately (plausibly due to the increase in the use of AI agents). There seems to be no good solution except to simply skip the test in CI.
There was a problem hiding this comment.
Code Review
This pull request introduces a skip condition for live PubChem API tests when running in CI environments and reduces the retry count for these tests. Feedback recommends making the CI detection more robust by checking for the variable's existence rather than a specific string value and suggests reverting the retry count reduction to maintain test stability during local execution.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
I don't know why I thought it would be better to reduce it to 4.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request updates 'src/openfermion/chem/pubchem_test.py' to skip live API tests when running in a CI environment. It introduces a 'skip_in_ci' decorator that checks for the 'CI' environment variable and applies it to the 'test_geometry_from_pubchem_live_api' test case to prevent failures caused by busy PubChem servers. Additionally, it reorders imports and adds the 'os' module. I have no feedback to provide.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request updates the PubChem test suite to skip live API tests when running in a CI environment. It introduces an import for the os module and defines a skip_in_ci decorator to prevent intermittent test failures caused by server availability issues. I have no feedback to provide.
pavoljuhas
left a comment
There was a problem hiding this comment.
LGTM after addressing inline suggestions.
Co-authored-by: Pavol Juhas <pavol.juhas@gmail.com>
As suggested by Pavol in review.
Thanks for those great suggestions. |
Even with many retries and long delays, the test fails for almost every PR. I don't know why this has been happening lately; perhaps Pubchem's servers have become overloaded lately (possibly due to an increase in the use of AI agents?). In any case, there seems to be no good solution to stop flaky failures in CI except to simply skip the test in CI.
The approach implemented here tests an environment variable set by GitHub in their runner environments. If that variable is set, the live API test is skipped; otherwise, it's run as usual. This means that developers in their normal local environments will run the tests as usual, and don't have to do anything special. In addition, our CI workflows don't need to change either.