Add get followers test #604
Conversation
|
|
|
Now I confirmed new test added! |
.github/workflows/php-checks.yml
Outdated
| - name: Generate code | ||
| run: | | ||
| export PATH=$PATH:~/bin/openapitools/ | ||
| bash tools/gen-oas-client.sh | ||
|
|
There was a problem hiding this comment.
I think that this code is unnecessary. This is because it causes a discrepancy with the actual commit content, leading to tests being conducted in that state, and the code generated by this script is not reflected in the PR. Since the differences generated by oas are submitted in a separate PR by diff-check.yaml, it seems unnecessary to do it here.
There was a problem hiding this comment.
Since your change is finally deleted here(
) as well, I cannot accept deleting this.Isn't the deletion of existing code during code generation itself problematic?
There are two options for a solution:
- Remove only the script that deletes existing code, but keep the call to
gen-oas-client.sh. - Move the location where test files are placed to somewhere other than
./src/clients/<project>.
There was a problem hiding this comment.
Out of the options mentioned above, I think option 1 is preferable.
Option 2 does not fundamentally solve the problem, and other contributors might encounter the same issue.
However, this would mean that tests are conducted in a state where there are newly added or edited files that are not reflected in the differences shown in the PR. Is this acceptable?
There was a problem hiding this comment.
Since your change is finally deleted here as well, I cannot accept deleting this.
The reason I allowed the execution of gen-oas-client.sh in diff-check.yml is that the deleted or edited code is displayed as a difference in the PR, ensuring that tests are not conducted in an unintended state. However, I agree, it is not ideal for the code to be deleted or edited in the first place 😅
There was a problem hiding this comment.
However, this would mean that tests are conducted in a state where there are newly added or edited files that are not reflected in the differences shown in the PR. Is this acceptable?
No it's not acceptable.
In this case, it would be appropriate to check for any differences using a separate job.
When there are differences other than those in the PR, marking the CI results as failed will alert us that there is invisible extra diff.
There was a problem hiding this comment.
I've implemented it with option 2 after all.
- If I eliminate
rm -rf $REPO_ROOT_DIR/src/clients/$schema, we won't be able to update the deleted items within the auto-generated code. In other words, no code other than the auto-generated one should be placed underclients/$schema. - Generally, in PHP, tests are often placed in tests/unit at the same level as the src directory. FYI
| @@ -6,6 +6,58 @@ on: | |||
| merge_group: | |||
|
|
|||
| jobs: | |||
| diff-check: | |||
There was a problem hiding this comment.
did you check if it works as expected?
There was a problem hiding this comment.
This reverts commit 1e8262c.
client.getFollowers(null, 99)must issue a request url like/v2/bot/followers/ids?limit=99. There must not be start as request parameter. This change adds a test to verify this.