-
Notifications
You must be signed in to change notification settings - Fork 24
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
channel list output is confusing #755 #777
Conversation
if (fullList) { | ||
console.println(ChannelMapper.toYaml(channels)); | ||
} else { | ||
console.println(channel.getName() + " " + channel.getManifestCoordinate().getGroupId() + ":" + channel.getManifestCoordinate().getArtifactId() + ":" + |
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.
This needs to handle following cases:
- Manifest coordinate is a Maven coordinate with full GAV
- Manifest coordinate is a Maven coordinate with just GA
- Manifest coordinate is an URL
- There is no manifest coordinate at all (we should print the value no-stream-strategy and ids of the repositories in this case)
prospero-cli/src/main/java/org/wildfly/prospero/cli/commands/ChannelCommand.java
Show resolved
Hide resolved
@@ -191,7 +191,33 @@ public void testList() { | |||
int exitCode = commandLine.execute(CliConstants.Commands.CHANNEL, CliConstants.Commands.LIST, | |||
CliConstants.DIR, dir.toString()); | |||
Assert.assertEquals(ReturnCodes.SUCCESS, exitCode); | |||
Assert.assertEquals(3, getStandardOutput().lines().filter(l->l.contains("manifest")).count()); | |||
Assert.assertEquals(3, getStandardOutput().lines() |
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.
Use Assertions.assertThat
and compare it to actual expected lines ("test1 g:a", "test2 g:a:v", "test3 file:/a:b")
CliConstants.DIR, dir.toString()); | ||
Assert.assertEquals(ReturnCodes.SUCCESS, exitCode); | ||
String output = getStandardOutput(); | ||
Assert.assertTrue("Expected 'schemaVersion' in the output", |
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 test doesn't actually verify that the correct values are printed. You should be able to get the output, convert it back to Channel objects and compare with the input.
Or simply compare the output with serialized input.
@spyrkob, I have updated the code according to the review comments. Please have a look and let me know if more changes are needed. |
} | ||
} else { | ||
// No manifest coordinate, print no-stream-strategy and repository ids | ||
console.println(channel.getName() + " no-stream-strategy: " + channel.getNoStreamStrategy()); |
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.
just one thing - can you inline this to something like <channel_name> <no_manifest_strategy> @ <repo1_id>,<repo2_id>[...]
. All other cases are one line, we should keep this the same
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.
Thanks @parsharma
Issue: #755