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

fix(sdk): include some missing properties in request body in transforming #8297

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

ihexxa
Copy link
Contributor

@ihexxa ihexxa commented Jan 15, 2025

Also with some minor fixes.

Changes

  • Request Body: in transforming, included some request body properties, such as multiline, fileName and disabled
  • Console: support escape sequences such as \n and print params in one line (instead of multiple lines)
  • Reject setting null to environment
  • In test output, also show completed version of Actual value and Expected value, originally it is truncated.

Multipart body

Screenshot 2025-01-15 at 15 36 01

log output

Screenshot 2025-01-17 at 14 11 22

In test, actual got value is 200, and expected value is a long string.

Screenshot 2025-01-16 at 17 10 35

Ref: INS-4795, #8240

@ihexxa ihexxa self-assigned this Jan 15, 2025
@ihexxa ihexxa changed the title fix: include some missing properties in request body in transforming fix(sdk): include some missing properties in request body in transforming Jan 15, 2025
@ihexxa ihexxa requested a review from a team January 15, 2025 08:18
@@ -22,6 +23,10 @@ export class Environment {
};

set = (variableName: string, variableValue: boolean | number | string) => {
if (variableValue === null) {
Copy link
Member

Choose a reason for hiding this comment

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

The varaibleValue seems inconsistent with the type definition.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, updated.

@@ -14,8 +14,11 @@ export class Console {
// TODO: support replacing substitution
printLog = (rows: Row[], level: LogLevel, ...values: any) => {
try {
const content = values.map((a: any) => JSON.stringify(a, null, 2).replace('\\n', '\n'))
Copy link
Member

Choose a reason for hiding this comment

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

When will it include \\n?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

JSON.stringify may transform \n into \\n in a (if it is a string).

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.

2 participants