test(fal): wait for e2e app alias readiness#1058
Conversation
badayvedat
force-pushed
the
vedat/fix-e2e-app-alias-readiness
branch
3 times, most recently
from
e7a3abc to
fad637d
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fad637de90
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
badayvedat
force-pushed
the
vedat/fix-e2e-app-alias-readiness
branch
from
fad637d to
ca45c41
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca45c411eb
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| pytest -n auto -v projects/fal/tests/integration \ | ||
| -k "not test_regular_function_in_a_container_with_custom_image" |
There was a problem hiding this comment.
In the checked workflow matrix this job also runs on windows-2022, where GitHub Actions uses PowerShell by default; PowerShell does not treat \ as a line continuation, so this step invokes pytest ... \ and then tries to run a separate -k ... command. The new custom-image step below has the same pattern. Keep these commands on one line or set shell: bash/use PowerShell continuation so the Windows matrix does not fail before running the tests.
Useful? React with 👍 / 👎.
badayvedat
force-pushed
the
vedat/fix-e2e-app-alias-readiness
branch
5 times, most recently
from
e04b248 to
1767e6f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1767e6fd93
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
badayvedat
force-pushed
the
vedat/fix-e2e-app-alias-readiness
branch
from
1767e6f to
2f9235c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f9235c02b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| # Create test environment | ||
| client.environments.create(test_env_name, description="Test env for secrets") | ||
| _wait_for_environment(client, test_env_name, present=True) |
There was a problem hiding this comment.
When environment propagation takes longer than the wait timeout, _wait_for_environment(..., present=True) returns (None, environments) rather than failing; this call ignores that result and immediately creates a secret in the environment. In that delayed-propagation scenario the test can still fail on client.secrets.set(...) because it proceeds with an environment that was never observed, so assert the returned environment is not None before continuing.
Useful? React with 👍 / 👎.
| if time.monotonic() >= deadline: | ||
| return runner_ids |
There was a problem hiding this comment.
When list_alias_runners() never satisfies the supplied condition before the deadline, this helper returns the last runner set anyway. In test_rollout_application the first wait can therefore return an empty set if the rollout leaves no replacement runner after 30s, and the subsequent runner_id_before not in runner_ids_after assertion passes even though the replacement never appeared; make the helper raise or have callers assert the condition was met.
Useful? React with 👍 / 👎.
badayvedat
force-pushed
the
vedat/fix-e2e-app-alias-readiness
branch
from
2f9235c to
20bcccf
Compare
1 participant
No description provided.