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

chore: refactor session detail storage & retrieval #1942

Merged
merged 12 commits into from
Feb 16, 2025

Conversation

eglitise
Copy link
Collaborator

This is primarily a refactoring PR for the Session Information screen, but also adjusts the data format used when storing the initial session details, which is also used in the Session Information screen:

  • Refactor Session Information
    • Extract the getSession call into a separate method for consistency
    • Avoid magic numbers when calculating session length
    • From user perspective, the server host, path, port, and session ID fields have been removed, but the session URL field has always included this information anyway
  • Assemble and store the full server URL along with the server/session details
  • Store headers in server details (currently only used for connecting to a TV Labs server)
  • Store a new boolean var isUsingMjpegMode instead of implicitly converting mjpegScreenshotUrl to boolean

@github-actions github-actions bot added chore Internal changes not visible to the user i18n Translation changes labels Feb 16, 2025
desiredCapabilities = addCustomCaps(desiredCapabilities);
let sessionCaps = originalCaps ? getCapsObject(originalCaps) : {};
let host, port, username, accessKey, https, path, headers;
sessionCaps = addCustomCaps(sessionCaps);

switch (session.serverType) {
case SERVER_TYPES.LOCAL:
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe in the future it makes sense to modularize this switch, so we keep all vendor-specific capability transformers (as well as other vendor-specific stuff) in separate modules, for example, sauce.js or perfecto.js, which must implement one common base interface (defined by us)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I fully agree, but that would also make this PR a lot bigger, so I would prefer to have a separate PR for that

Copy link
Contributor

Choose a reason for hiding this comment

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

ofc, not in the same PR

@eglitise eglitise merged commit a32a339 into appium:main Feb 16, 2025
7 checks passed
@eglitise eglitise deleted the refactor-session-data branch February 16, 2025 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Internal changes not visible to the user i18n Translation changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants