Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial fixes to restore Java IT #5632

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

andreaTP
Copy link
Contributor

@andreaTP andreaTP commented Oct 18, 2024

This PR is only fixing the most evident issues with the Java IT tests.
fixes #5610

@andreaTP andreaTP requested a review from a team as a code owner October 18, 2024 09:43
@@ -21,7 +21,7 @@ if ($language -eq "csharp") {
$command = " --output `"./it/$language/client`" --namespace-name `"app.client`""
}
elseif ($language -eq "java") {
$command = " --output `"./it/$language/src`""
$command = " --output `"./it/$language/src/apisdk`""
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is, likely, another breaking change of the "default behavior".

Copy link
Member

Choose a reason for hiding this comment

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

meaning it'll break other integration tests or did you mean something else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I implemented the IT tests, the CLI was correctly placing the files in the folder matching the package name, now it's not.

I haven't bisected the issue to see what changed, though, was the previous "default package" null?

Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

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

I went ahead and merged my earlier PR #5624 to avoid having competing work. Updated this one from main.
It seems that all those changes do not solve the parsable issue.
https://github.com/microsoft/kiota/actions/runs/11401393719/job/31726688511?pr=5632

@@ -21,7 +21,7 @@ if ($language -eq "csharp") {
$command = " --output `"./it/$language/client`" --namespace-name `"app.client`""
}
elseif ($language -eq "java") {
$command = " --output `"./it/$language/src`""
$command = " --output `"./it/$language/src/apisdk`""
Copy link
Member

Choose a reason for hiding this comment

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

meaning it'll break other integration tests or did you mean something else?

@baywet
Copy link
Member

baywet commented Oct 18, 2024

ok so after looking more into it, all the parsable errors can be ignored. They are a side effect of the circular reference for the integration model.

@baywet
Copy link
Member

baywet commented Oct 18, 2024

(I'll keep sharing my findings on the original issue)

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.

Breaking changes introduced in 1.15
2 participants