Skip to content

feat: Add fileFieldName default value of ScreenRecordingUploadOptions#1591

Closed
jaden-k wants to merge 1 commit into
appium:masterfrom
jaden-k:master
Closed

feat: Add fileFieldName default value of ScreenRecordingUploadOptions#1591
jaden-k wants to merge 1 commit into
appium:masterfrom
jaden-k:master

Conversation

@jaden-k

@jaden-k jaden-k commented Nov 16, 2021

Copy link
Copy Markdown

Change list

Added fileFieldName default value(file) of ScreenRecordingUploadOptions with some basic tests

Types of changes

  • No changes in production code.
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Details

In v7.4.0 earlier versions,STOP_RECORDING_SCREEN command is not working since appium-server v1.21.0.
One of the ways to solve this problem is upgrading java-client and adding the fileFieldName option in test code.
It looks like being provided fileFieldName default value is the better one.

@linux-foundation-easycla

linux-foundation-easycla Bot commented Nov 16, 2021

Copy link
Copy Markdown

CLA Signed

The committers are authorized under a signed CLA.

private Map<String, Object> formFields;

/**
* Creates new instance with default fileFieldName.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Right~ but, 'Content-Length' header is added by this line.
Probably, it would occur a problem again that fileFieldName option is set to default value before this line.
So, I made this PR.
Please let me know if you have any better ideas.

@mykola-mokhnach mykola-mokhnach Nov 17, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

then it would make sense to change the way headers are set. I'll create a PR for it. You could anyway provide a value to withFileFieldName in order to workaround the issue with the existing implementation

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

appium/appium-support#233 should do the trick

@jaden-k

jaden-k commented Nov 17, 2021

Copy link
Copy Markdown
Author

uncalled-for~
appium-support(appium server) provide this feature

@jaden-k jaden-k closed this Nov 17, 2021
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.

3 participants