ci: Replace APIGurus as source for IT#7774
Conversation
|
Hope I fixed the failing tests. I had not seen that "integration-tests.yml" and "idempotency-tests.yml" define the old api guru URLs for preloading. |
|
@WolfgangHG I think something is off with artifacts download/upload and key composition. see https://github.com/microsoft/kiota/actions/runs/27542430722/job/81407904278 |
|
@baywet It seems the docusign json file was not download: https://github.com/microsoft/kiota/actions/runs/27542430722/job/81407737625 ( |
|
@WolfgangHG my guess is that this is not specific to this description, but instead this script is silently failing. kiota/it/download-openapi-specs.ps1 Line 45 in aef1960 GitHub typically throttles unauthenticated requests much more than authenticated ones. Kiota automatically does retrial.
Let me know if you have any additional comments or questions. |
|
I think the Invoke-WebRequest prints error messages if something goes wrong. But I don't see anything in the output. I could add diagnostic output to |
|
I've seen many instances where the output behaviour differs on PowerShell between the local and the GHA run environment. And you end up not "seeing" things. So yes, adding additional logging would be a good way to start. |
Head branch was pushed to by a user without write access
|
I added several And I added ...because when calling the script from a CMD window, the exit code ( @baywet Could you start another CI run? |
|
Rather obvious: the upload step always tried to upload "openapi.yaml", but with my changes, there could also be a "openapi.json". Shame on me that I did not see this before ;-). Now I try to upload both files - let's see what happens. Hope that the file is not present from a previous step. Maybe we should delete both files "openapi.json" and "openapi.yaml" in "download-openapi-specs.ps1". Same fix should be applied to "integration-tests.yml". And about logging: @baywet Another CI run please. |
|
Seems the upload of both openapi.json and openapi.yaml in one step works. New error happens in ./it/compare-generation.ps1 (https://github.com/microsoft/kiota/actions/runs/27572392069/job/81636616228), which always accessed "openapi.yaml" and thus failed with json files. @baywet I committed a fix, could you start CI again? |
|
Another change: "get-description-artifact-key.ps1" always returned @baywet it is your turn again. And all of this only because I changed "openapi.yaml" to "openapi.json" for json files (as part of the "default values" changes) - I considered it wrong to have a json file with wrong extension though this did not cause problems when invoking kiota ;-) |
|
I think we make progress - this time most idempotency tests worked. I applied the fix from "idempotency-tests.yml" also to "integration-tests.yml": after downloading the api description, try also to upload "openapi.json". @baywet One more time.... One of the failing idempotency tests seems to have run into a real error, maybe due to a newer api description.. |
|
@baywet I digged through the idempotency tests. Here are the errors: Problem 1: ==>There was a change in file "models\partial_user_object_response.py": @dataclass
class PartialUserObjectResponse(Parsable):
"""
The user who created the data source.
"""Second one: @dataclass
class PartialUserObjectResponse(Parsable):
"""
User who created the page.
"""Maybe there was a change to the online api descriptions between test run 1 and 2. But this should not happen, as the test uses one single artifact that was downloaded before by this job: https://github.com/microsoft/kiota/actions/runs/27618422969/job/81665975879. Problem 2: This is probably a new error due to changes in the api description since the apigurus version. Do we add a exclusion? Problem 3: Same question as problem 2.... |
|
Hi @WolfgangHG Looking at the CI results, it seems the issues are not just limited to TypeScript and Python? Do you have more context here? or is the python class description issue propagated to more languages? It'd be interesting to share the component definition here, and the references to it to understand how does the generator get into that race condition. For TypeScript, to me that sounds like a bug in the TypeScript generation, I'd log a detailed bug and suppress for later investigation. Let me know if you have any additional comments or questions. |
|
Added suppression/exclusion for the two failing typescript clients. @baywet Could you start another CI run? Maybe the python idempotency problem was a random error. |
|
I worked through six more apis/language errors and added suppressions/exclusions. For Java/Github api, there is mock server test ("\it\java\gh\src\test\java\GHAPITest.java") that did not compile with recent api. I think I fixed it - Github apparently changed a parameter name. Also reverted the NOTION.com/PHP suppression after the fix of #7809 @baywet Could you give CI another run? There should still be five or six errors remaining. |
WolfgangHG
force-pushed
the
apigurus
branch
2 times, most recently
from
cb0abd2 to
8c5369d
Compare
|
@baywet I think I am done with all failing tests and flooded the repo with a lot of new issues ;-) |
|
Last CI run failed with the test "typescript and GITHUB api" - I had a typo in the suppression rule. This is fixed now - another test run please 😄 ! |
|
@WolfgangHG it seems there is a conflict that needs to be resolved before we can run tests |
WolfgangHG
force-pushed
the
apigurus
branch
2 times, most recently
from
36f3157 to
9408fe9
Compare
|
@gavinbarron I rebased. The conflicting change from #7859 to "it/Readme.md" introduced a chapter "Surface area / DOM diff test" that was put in the middle of the integration test description (the next chapter "Mock server tests" belongs to "Running the Integration Tests"). It seems that "Surface area / DOM diff test" is not related to the integration tests stuff, so I suggest to move it either to the beginning or the end of the file. What do you think? |
|
it's an integration test for another purpose, putting it to the end of the of the file makes sense to me. |
|
The failing idempotency test https://github.com/microsoft/kiota/actions/runs/28884299669/job/85681452557 shows exactly the same error that happened three weeks ago, see analysis in #7774 (comment) (section "Problem 1"), even with the same code diff. |
|
Yeah, rerunning that job got it to pass, one thing that might help isolate what's going on with that flaky test is to output the kiota lock files when there is a failure. |
|
@gavinbarron I changed the section order in "it/Readme.md" and moved the "Surface area / DOM diff test" part to the end. Hope it is OK for you? As in the previous test run a bunch of other NOTION.COM idempotency tests failed, I added code to "compare-generation.ps1" (with the help of Copilot) that reports the files that are different. Hope this makes it easier to track down the differences next time. About your comment
Previously, those files were deleted before computing the hash. I changed the code and added them to the exclusion of Could you start another test run, just to see whether an idempotency test fails and whether my new code helps analyzing the problem? Maybe I introduced an error when changing the download urls and missed something.... |
|
The recent changes to "compare-generation.ps1" revealed that "kiota-lock.json" contains the same description hash for both files, though there are differences in the result. So, the same API description was picked for both runs. But I think I found the reason for the idempotency errors and created #7927 - could we continue discussion there? |
|
For the logs, instead of uploading them in the zip, which is going to make them difficult to access, we should instead merge them into the action logs with something like this - name: Run tool and merge logs on failure
if: always()
run: |
if [ -f my-file.log ]; then
echo "::group::Tool Logs (Post-Execution)"
cat my-file.log
echo "::endgroup::"
fi |
3 participants
This pull request contains the changes suggested in #7495: the OpenAPI specs hosted at APIGurus do not seem to be updated any longer. So pull the recent api specs from official sources where possible.
What do you think? Is this reasonable?
This makes sense in my opinion because it ensures that Kiota is compatible to the most recent version of commonly used APIs (e.g. Twitter). But it has the drawback that errors in the API might cause false errors in Kiota IT, and changes might require additional changes in Kiota.
I also checked the suppressions and exclusions and removed them if I could not reproduce the error (either fixed in a newer API version or a Kiota change resolved it). But maybe I removed too much, if e.g. the error reason was not meaningful enough. So, several tests might fail if this pull request performs the first CI run.
The linked issues in Ruby suppressions did not always make the actual error clear, so maybe I removed too much or the updated issue link does not point to the original error.
I also found one API that should ideally be pulled from Postman (#7495 (comment)), but this is not part of this pull request.