Skip to content

Fix broken sample and add test for sample #1216

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Sep 20, 2018
Merged

Fix broken sample and add test for sample #1216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file added video/cloud-client/resources/cat.mp4
View file
Open in desktop
Binary file not shown.
4 changes: 1 addition & 3 deletions video/cloud-client/src/main/java/com/example/video/Detect.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,13 +179,11 @@ public static void analyzeLabelsFile(String filePath) throws Exception {
// Read file and encode into Base64
Path path = Paths.get(filePath);
byte[] data = Files.readAllBytes(path);
byte[] encodedBytes = Base64.encodeBase64(data);

AnnotateVideoRequest request = AnnotateVideoRequest.newBuilder()
.setInputContent(ByteString.copyFrom(encodedBytes))
.setInputContent(ByteString.copyFrom(data))
.addFeatures(Feature.LABEL_DETECTION)
.build();

// Create an operation that will contain the response when the operation completes.
OperationFuture<AnnotateVideoResponse, AnnotateVideoProgress> response =
client.annotateVideoAsync(request);
Expand Down
14 changes: 12 additions & 2 deletions video/cloud-client/src/test/java/com/example/video/DetectIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ public class DetectIT {
private ByteArrayOutputStream bout;
private PrintStream out;

static final String LABEL_FILE_LOCATION = "gs://demomaker/cat.mp4";
static final String LABEL_GCS_LOCATION = "gs://demomaker/cat.mp4";
static final String LABEL_FILE_LOCATION = "./resources/cat.mp4";
static final String SHOTS_FILE_LOCATION = "gs://demomaker/gbikes_dinosaur.mp4";
static final String EXPLICIT_CONTENT_LOCATION = "gs://demomaker/cat.mp4";

Expand All @@ -51,7 +52,16 @@ public void tearDown() {

@Test
public void testLabels() throws Exception {
String[] args = {"labels", LABEL_FILE_LOCATION};
String[] args = {"labels", LABEL_GCS_LOCATION};
Detect.argsHelper(args);
String got = bout.toString();
// Test that the video with a cat has the whiskers label (may change).
assertThat(got.toUpperCase()).contains("WHISKERS");
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be worthwhile to update these tests to check that some labels were successfully returned, instead of a specific label (unless the docs specifically mention it should be detecting that label)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Like switching the functions to return the result and then check if the size of returned labels are not 0?

I know that for ML APIs, I believe we still want to validate some of the results to make sure the results are accurate. But let me go double check.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just something to consider if the labels change a lot. Not blocking PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Merging for now, will check with team on testing changes.

Copy link

Choose a reason for hiding this comment

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

+1 @kurtisvg Love the idea!

This came up in conversation only last week for testing responses from Machine Learning APIs based on models that can and do change over time

I love the idea of providing a list of test inputs/outputs and verifying that at least 1 of the assertions passed

It is very much à la Data Driven Tests aka Parameterized Tests

(see: JUnit4 Theories & other xUnit-style frameworks which implement 'Theories' for data-driven tests – C# xUnit Theory, Go Table-Driven-Tests, Python parameterized package, ...)

The main difference versus typical parameterized tests is that they usually exist to verify ALL SCENARIOS SUCCEED (versus any one of many scenarios succeed).

pseudocode example of parameterized tests

method testMySample
  run_sample "input.png"

  assert output includes "Label: Dog"
end

^--- rather than 1 test that asserts "Dog" is returned as a label when analyzing an image file, provide a LIST of input/outputs to test

test_cases = [
  [ "dog.png", "Label: Dog" ],
  [ "cat.png", "Label: Cat" ],
  [ "cat.png", "Label: Feline" ]
]

method data_driven_test file_name, output_assertion
    run_sample file_name

  assert output includes output_assertion
end

And we want to assert at least one of the test cases passes

I love this. But I also want to be careful implementing it at this time. Because we have test-case consistency across 7 languages. When we start using this technique, I want to be clear about the expectations across languages. This also adds a new responsibility to the author of the canonical sample: rather than coming up with 1 test case, they will be tasked with coming up with multiple which should be stable over time – these should probably come from the product team.

#misc: I'm in the design phases of a generator for code sample tests (initially only for code samples that are generated as well) and this is one of my design requirements. I am backing in table/data-driven tests with the toggle option of either verifying that ALL SUCCESS (default) or ANY ONE SUCCEEDS (available option)

}

@Test
public void testLabelsFile() throws Exception {
String[] args = {"labels-file", LABEL_FILE_LOCATION};
Detect.argsHelper(args);
String got = bout.toString();
// Test that the video with a cat has the whiskers label (may change).
Expand Down