-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Fix the sleep logic for streaming as per sample rate #279
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
Changes from all commits
80eb6da
7d8f4b5
d87d691
ff7ffbc
a23abdc
0cee557
df11062
a253ebf
d1fdc61
a44e692
4ac456e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,87 @@ | ||
/* | ||
* Copyright 2016 Google Inc. All Rights Reserved. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
package com.examples.cloud.speech; | ||
|
||
import static com.google.common.truth.Truth.assertThat; | ||
|
||
import io.grpc.ManagedChannel; | ||
import org.apache.log4j.Logger; | ||
import org.apache.log4j.SimpleLayout; | ||
import org.apache.log4j.WriterAppender; | ||
import org.junit.After; | ||
import org.junit.Before; | ||
import org.junit.Test; | ||
import org.junit.runner.RunWith; | ||
import org.junit.runners.JUnit4; | ||
|
||
import java.io.File; | ||
import java.io.IOException; | ||
import java.io.StringWriter; | ||
import java.io.Writer; | ||
import java.net.URI; | ||
import java.nio.file.Path; | ||
import java.nio.file.Paths; | ||
|
||
|
||
/** | ||
* Unit tests for {@link StreamingRecognizeClient }. | ||
*/ | ||
@RunWith(JUnit4.class) | ||
public class StreamingRecognizeClientTest { | ||
private Writer writer; | ||
private WriterAppender appender; | ||
|
||
@Before | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have to do it for each test so that StringWriter is new. There is an After which I now think should be AfterClass where all added appenders will be removed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh I see. No - having it in an After makes sense. Sorry - didn't see the After. |
||
public void setUp() { | ||
writer = new StringWriter(); | ||
appender = new WriterAppender(new SimpleLayout(), writer); | ||
Logger.getRootLogger().addAppender(appender); | ||
} | ||
|
||
@After | ||
public void tearDown() { | ||
Logger.getRootLogger().removeAppender(appender); | ||
} | ||
|
||
@Test | ||
public void test16KHzAudio() throws InterruptedException, IOException { | ||
URI uri = new File("resources/audio.raw").toURI(); | ||
Path path = Paths.get(uri); | ||
|
||
String host = "speech.googleapis.com"; | ||
int port = 443; | ||
ManagedChannel channel = AsyncRecognizeClient.createChannel(host, port); | ||
StreamingRecognizeClient client = new StreamingRecognizeClient(channel, path.toString(), 16000); | ||
|
||
client.recognize(); | ||
assertThat(writer.toString()).contains("transcript: \"how old is the Brooklyn Bridge\""); | ||
} | ||
|
||
@Test | ||
public void test32KHzAudio() throws InterruptedException, IOException { | ||
URI uri = new File("resources/audio32KHz.raw").toURI(); | ||
Path path = Paths.get(uri); | ||
|
||
String host = "speech.googleapis.com"; | ||
int port = 443; | ||
ManagedChannel channel = AsyncRecognizeClient.createChannel(host, port); | ||
StreamingRecognizeClient client = new StreamingRecognizeClient(channel, path.toString(), 32000); | ||
|
||
client.recognize(); | ||
assertThat(writer.toString()).contains("transcript: \"how old is the Brooklyn Bridge\""); | ||
} | ||
} |
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.
Shouldn't this be configured in a configuration file, rather than in code? eg if a user wanted to log to a file instead of stdout..
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.
It generally is, but I thought for the purpose of sample was bit of an overkill. But I can change if folks have other thoughts.
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 letting the logger log to the default destination is fine for the sample. If there's a reason that messages should go to stdout, then it should print to stdout rather than logging.
Semantics are important for samples. If you're logging something, then it should be something that shows up in logs. If you're printing something, then it should be communicating with the user. Sending logs to stdout sends sort of mixed messages, IMO.
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 the problem here is different. Which is we are inspecting the logs to evaluate the tests, which is not a generally we should do. Its because of this reason. Now, we shouldn't do System.out.println in the Streaming sample and therefore we have to do this trick in the JUnit Tests. One way I think of fixing this is by having the recognize method return a Stream so tha JUnit inspects the Stream and now you see its getting bit involved :)
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.
Oh interesting...
So this script doesn't even display any output, it just logs the response... -_-; This strikes me as a bit weird, but I realize it's tangential to this CL, so I won't block on it. I do like your idea about returning a Stream, but yeah. Another time :-)
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.
Usually, we just configure this with logging.properties, you could comment. If you are going to run this on GCE, you might wish to integrate w/ gcloud-java's logging.