[FLINK-2480][test]Add tests for PrintSinkFunction #991

HuangWHWHW · 2015-08-05T03:04:46Z

Test PrintSinkFunction:
set number of subtasks with 0 in runtime ctx to make prefix null.
1.set STD.OUT and test print system.out
2.set STD.ERR and test print system.err

HuangWHWHW · 2015-08-05T03:28:33Z

I still cannot see the CI.
Does any one can help to support this CI info??

fhueske · 2015-08-05T09:33:46Z

...aming-core/src/test/java/org/apache/flink/streaming/api/functions/PrintSinkFunctionTest.java

+import org.apache.flink.api.common.ExecutionConfig;
+import org.apache.flink.api.common.JobID;
+import org.apache.flink.api.common.accumulators.Accumulator;
+//import org.apache.flink.api.common.functions.RuntimeContext;


Please remove the imports which are commented out.

HuangWHWHW · 2015-08-05T12:34:38Z

@fhueske
Thank you!I`ll fix it.

mxm · 2015-08-12T13:10:49Z

Your pull request doesn't compile: https://s3.amazonaws.com/archive.travis-ci.org/jobs/74504427/log.txt

mxm · 2015-08-13T15:53:27Z

...aming-core/src/test/java/org/apache/flink/streaming/api/functions/PrintSinkFunctionTest.java

+		printSink.setTargetToStandardOut();
+		printSink.invoke("hello world!");
+
+		assertEquals("Print to System.out", printSink.toString());


You're not testing whether the sink actually prints to stdout. You're just checking if the toString() methods returns the correct mode.

As I see, the sink actually prints is printSink.invoke("hello world!");.
And in the function invoke, it just calls stream.println(record.toString());.
Could you tell me how to get the prints from the stream?
Because I think assertEquals("hello world!", record); is just like assertEquals("hello world!", "hello world!"); and it`s not necessary.

Sorry, ignore this.
I just have got the way.

You can replace stdout using System.setOut(..) by a PrintStream which writes to a ByteArrayOutputStream.

Yes, I did this in my new change.

Ah, didn't see that. Great!

HuangWHWHW · 2015-08-14T07:03:39Z

Hi, I have done a new changes.

mxm · 2015-08-25T08:50:49Z

...aming-core/src/test/java/org/apache/flink/streaming/api/functions/PrintSinkFunctionTest.java

+	public void testPrintSinkStdOut(){
+
+		printStreamMock stream = new printStreamMock(out);
+		System.setOut(stream);


Could you save and restore System.out in an @After shutdown method?

Hi, I add this in my new change.

mxm · 2015-08-25T12:11:32Z

Thanks for the updates! Looks good and I think we can merge your changes.

HuangWHWHW · 2015-08-25T12:16:31Z

@mxm
Thank you.
Sorry for that I haven`t updated the code in this branch for a long time.
So you can wait the CI to pass.

StephanEwen · 2015-08-25T15:44:15Z

@HuangWHWHW Can you access the CI reports now? Has the Travis team fixed the problem?

HuangWHWHW · 2015-08-26T01:00:36Z

@StephanEwen
Hi,
Not yet.
I will ask the travis support again.

mxm · 2015-08-26T08:57:13Z

...aming-core/src/test/java/org/apache/flink/streaming/api/functions/PrintSinkFunctionTest.java

+
+	@After
+	public void restoreSystemOut() {
+		System.setOut(new PrintStream(new FileOutputStream(FileDescriptor.out)));


Could you restore the original PrintStream here? You can save it in a variable like PrintStream original = System.out and restore it here.

Ok, I add this in my new change.

mxm · 2015-08-26T09:35:44Z

...aming-core/src/test/java/org/apache/flink/streaming/api/functions/PrintSinkFunctionTest.java

+		}
+	};
+
+	public PrintStream PrintStreamOriginal = null;


This should be private PrintStream PrintStreamOriginal = System.out;. Then you don't need the null check later on and simply restore it in the @After method.

Yes, fixed these two comments.

mxm · 2015-08-26T10:04:55Z

...aming-core/src/test/java/org/apache/flink/streaming/api/functions/PrintSinkFunctionTest.java

+ */
+public class PrintSinkFunctionTest<IN> extends RichSinkFunction<IN> {
+
+	public PrintStream PrintStreamOriginal = System.out;


By convention, the variable name should be lower case here.

AhAh......
Sorry, I get you now.

Changed public PrintStream PrintStreamOriginal = System.out; to private PrintStream printStreamOriginal = System.out; now. :)

HuangWHWHW · 2015-08-26T15:07:36Z

@mxm
Hi,
sorry for a careless again.
Now I fix the problem which the CI was failed.

mxm · 2015-08-26T15:27:18Z

Thanks @HuangWHWHW. I'll merge your changes when Travis has completed.

HuangWHWHW · 2015-08-27T02:38:30Z

@mxm
Hi,
the CI is pass.
Did it probability failure?

mxm · 2015-08-27T09:17:10Z

...aming-core/src/test/java/org/apache/flink/streaming/api/functions/PrintSinkFunctionTest.java

+		}
+	}
+
+	private Environment envForPrefixNull = new Environment() {


You can replace all this by private Environment envForPrefixNull = Mockito.mock(Environment.class).

mxm · 2015-08-27T09:22:07Z

Thank you @HuangWHWHW. I merged your pull request with a few minor changes (see new comments).

This closes apache#991.

coveralls · 2017-05-04T22:23:06Z

Changes Unknown when pulling fde7c49 on HuangWHWHW:FLINK-2480 into ** on apache:master**.

fhueske reviewed Aug 5, 2015
View reviewed changes

HuangWHWHW force-pushed the FLINK-2480 branch from 810fdde to d0ddfa8 Compare August 7, 2015 01:05

HuangWHWHW force-pushed the FLINK-2480 branch from d0ddfa8 to 14e4396 Compare August 13, 2015 06:20

HuangWHWHW mentioned this pull request Aug 13, 2015

[FLINK-2490][FIX]Remove the retryForever check in function streamFrom… #992

Closed

mxm reviewed Aug 13, 2015
View reviewed changes

HuangWHWHW force-pushed the FLINK-2480 branch 2 times, most recently from 4dbfc28 to 702b4b1 Compare August 14, 2015 06:58

mxm reviewed Aug 25, 2015
View reviewed changes

[FLINK-2480][streaming]update code

90e640e

HuangWHWHW force-pushed the FLINK-2480 branch from c26ae02 to 90e640e Compare August 25, 2015 12:13

mxm reviewed Aug 26, 2015
View reviewed changes

[FLINK][streaming]add a var to store System.out

b27bb83

mxm reviewed Aug 26, 2015
View reviewed changes

[FLINK-2480][test]change the var of system.out

1a59d90

mxm reviewed Aug 26, 2015
View reviewed changes

[FLINK-2480][TEST]fix some CI issue and rerun CI

fde7c49

HuangWHWHW force-pushed the FLINK-2480 branch from 115debc to fde7c49 Compare August 27, 2015 00:59

mxm reviewed Aug 27, 2015
View reviewed changes

asfgit closed this in d08733d Aug 27, 2015

nikste pushed a commit to nikste/flink that referenced this pull request Sep 29, 2015

[FLINK-2480][test] add test for PrintSinkFunction

9128243

This closes apache#991.

rmetzger added the component=API/DataStream label Mar 14, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FLINK-2480][test]Add tests for PrintSinkFunction #991

[FLINK-2480][test]Add tests for PrintSinkFunction #991

HuangWHWHW commented Aug 5, 2015

HuangWHWHW commented Aug 5, 2015

fhueske Aug 5, 2015

HuangWHWHW commented Aug 5, 2015

mxm commented Aug 12, 2015

mxm Aug 13, 2015

HuangWHWHW Aug 14, 2015

HuangWHWHW Aug 14, 2015

mxm Aug 24, 2015

HuangWHWHW Aug 25, 2015

mxm Aug 25, 2015

HuangWHWHW commented Aug 14, 2015

mxm Aug 25, 2015

HuangWHWHW Aug 25, 2015

mxm commented Aug 25, 2015

HuangWHWHW commented Aug 25, 2015

StephanEwen commented Aug 25, 2015

HuangWHWHW commented Aug 26, 2015

mxm Aug 26, 2015

HuangWHWHW Aug 26, 2015

mxm Aug 26, 2015

HuangWHWHW Aug 26, 2015

mxm Aug 26, 2015

HuangWHWHW Aug 26, 2015

HuangWHWHW Aug 26, 2015

HuangWHWHW commented Aug 26, 2015

mxm commented Aug 26, 2015

HuangWHWHW commented Aug 27, 2015

mxm Aug 27, 2015

mxm commented Aug 27, 2015

coveralls commented May 4, 2017

[FLINK-2480][test]Add tests for PrintSinkFunction #991

[FLINK-2480][test]Add tests for PrintSinkFunction #991

Conversation

HuangWHWHW commented Aug 5, 2015

HuangWHWHW commented Aug 5, 2015

Choose a reason for hiding this comment

HuangWHWHW commented Aug 5, 2015

mxm commented Aug 12, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HuangWHWHW commented Aug 14, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mxm commented Aug 25, 2015

HuangWHWHW commented Aug 25, 2015

StephanEwen commented Aug 25, 2015

HuangWHWHW commented Aug 26, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HuangWHWHW commented Aug 26, 2015

mxm commented Aug 26, 2015

HuangWHWHW commented Aug 27, 2015

Choose a reason for hiding this comment

mxm commented Aug 27, 2015

coveralls commented May 4, 2017