-
Notifications
You must be signed in to change notification settings - Fork 119
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
886 Print network profiles #906
Conversation
7de4d53
to
1727ec1
Compare
} returns configurationsMock | ||
every { configurationsMock.asPrintableTable() } returns "" | ||
CommandLine(NetworkProfilesListCommand()).execute() | ||
verify { configurationsMock.asPrintableTable() } |
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.
I think that we have response for this call-in mocked server. Could we use it somehow?
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.
I'm not 100% sure but probably I need to mock it to check printing is executed on command run.
Could you add also the possibility to run table using a command |
@@ -24,6 +26,7 @@ class AndroidTestEnvironmentCommand : Runnable { | |||
println(devicesCatalogAsTable(AndroidArgs.load(Paths.get(configPath)).project)) | |||
println(supportedVersionsAsTable(AndroidArgs.load(Paths.get(configPath)).project)) | |||
println(providedSoftwareAsTable()) | |||
println(GcTesting.networkConfiguration().asPrintableTable()) |
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.
Could you update description to :
header = ["Print available devices, OS versions, provided software list and network configuration to test against"],
description = ["Print available Android devices, Android OS versions list, provided software and network configuration to test against"],
?
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.
Changed
@@ -24,6 +26,7 @@ class IosTestEnvironmentCommand : Runnable { | |||
println(devicesCatalogAsTable(IosArgs.load(Paths.get(configPath)).project)) | |||
println(softwareVersionsAsTable(IosArgs.load(Paths.get(configPath)).project)) | |||
println(providedSoftwareAsTable()) | |||
println(GcTesting.networkConfiguration().asPrintableTable()) |
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.
Could you update description to :
header = ["Print available devices, OS versions, provided software list and network configuration to test against"],
description = ["Print available iOS devices, iOS OS versions list, provided software and network configuration to test against"],
?
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.
Changed
For me defaults separator should not be printed cause tables become larger (readability is a plus, but the size is a minus :( ) |
…code style on printing network configuration
So we thinking the same. I added a table style option and set default style without rows separation. Thanks for your feedback! |
Great work 👍 |
Fixes #886
Test Plan
Command
and
Should print table
also
should print this table at the end of the output.
WDYT
I added row separator to all tables to keep one style. In my opinion, it looks fine but on the other hand, the table with row separator takes more space. Tell me please what you think. Maybe we shouldn't add row separators to all tables.
Checklist
flank firebase test network-profiles list
flank firebase test android|ios test-environment