-
Notifications
You must be signed in to change notification settings - Fork 324
[http-client-java]mgmt, fix unix time mock #9111
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
base: main
Are you sure you want to change the base?
Conversation
| $tspOptions += " --option ""@typespec/http-client-java.uuid-as-string=false""" | ||
| } elseif ($tspFile -match "tsp[\\/]arm-stream-style-serialization.tsp") { | ||
| # for mgmt, do not generate tests due to random mock values | ||
| $tspOptions += " --option ""@typespec/http-client-java.generate-tests=false""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generate mock tests to at least ensure compilation pass. Generated tests will be git-ignored and won't be checked-in.
|
No changes needing a change description found. |
|
You can try these changes here
|
| private static ExampleNode defaultNode(IType clientType, IType wireType, Object exampleValue) { | ||
| ExampleNode node; | ||
| LiteralNode literalNode = new LiteralNode(clientType, exampleValue); | ||
| LiteralNode literalNode = new LiteralNode(clientType, wireType, exampleValue); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of add a wireType, is it solvable by adding a case in public static String convertLiteralToClientValue(IType wireType, String literalInWireType) ?
I think it kind of convert the objectValue to clientValue (e.g. in this case, convert the epoch to datetime string).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bug maybe there?
The code maybe need to be this
literalValue = Instant.ofEpochSecond(Long.parseLong(Integer.parseInt(literalValue)).atOffset(ZoneOffset.UTC).toString()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, we could do this. Though seems we still need wireType here. Any better ways to pass in the information?
Lines 120 to 131 in 9a47aae
| public void accept(ExampleNode node, String getterCode) { | |
| if (node instanceof LiteralNode) { | |
| node.getClientType().addImportsTo(imports, false); | |
| IType wireType = ((LiteralNode) node).getWireType(); | |
| addEqualsAssertion( | |
| wireType.convertToClientType(node.getObjectValue() == null | |
| ? wireType.defaultValueExpression() | |
| : wireType.defaultValueExpression( | |
| // We are already using wireType, thus wire value should be used, instead of client value. | |
| String.valueOf(node.getObjectValue()))), | |
| getterCode, node.getClientType().asNullable() == ClassType.BOOLEAN); |
Original code is:
Lines 120 to 125 in 51ed31e
| public void accept(ExampleNode node, String getterCode) { | |
| if (node instanceof LiteralNode) { | |
| node.getClientType().addImportsTo(imports, false); | |
| addEqualsAssertion(node.getClientType().defaultValueExpression(((LiteralNode) node).getLiteralsValue()), | |
| getterCode, node.getClientType().asNullable() == ClassType.BOOLEAN); |
Which is buggy.
wireTypeinformation forLiteralNodein assertions, since it contains encoding information.