> On Dec. 3, 2018, 8:23 p.m., Szabolcs Vasas wrote: > > src/test/org/apache/sqoop/util/TestMainframeFTPClientUtils.java > > Lines 343 (patched) > > <https://reviews.apache.org/r/62523/diff/10/?file=2110996#file2110996line378> > > > > I think the purpose of this test is to verify that if you create the > > FTP connection, the init commands are executed, right? > > In that case it would be better to modify it to use Mockito's verify > > functionality: > > ``` > > @Test > > public void testFtpCommandExecutes() throws IOException { > > final String EXPECTED_RESPONSE = "200 OK"; > > final int EXPECTED_RESPONSE_CODE = 200; > > String ftpcmds = "quote SITE RDW,quote SITE RDW READTAPEFORMAT=V"; > > final int FTP_CMD_COUNT = 2; > > when(mockFTPClient.login("user", "pssword")).thenReturn(true); > > when(mockFTPClient.logout()).thenReturn(true); > > when(mockFTPClient.isConnected()).thenReturn(false); > > > > when(mockFTPClient.getReplyCode()).thenReturn(EXPECTED_RESPONSE_CODE); > > when(mockFTPClient.getReplyString()).thenReturn(EXPECTED_RESPONSE); > > setupDefaultConfiguration(); > > conf.set(MainframeConfiguration.MAINFRAME_INPUT_DATASET_TYPE,"g"); > > > > conf.set(MainframeConfiguration.MAINFRAME_INPUT_DATASET_NAME,"a.b.c.d"); > > > > conf.set(MainframeConfiguration.MAINFRAME_FTP_TRANSFER_MODE,MainframeConfiguration.MAINFRAME_FTP_TRANSFER_MODE_BINARY); > > conf.set(MainframeConfiguration.MAINFRAME_FTP_CUSTOM_COMMANDS, > > ftpcmds); > > MainframeFTPClientUtils.setMockFTPClient(mockFTPClient); > > > > MainframeFTPClientUtils.getFTPConnection(conf); > > > > verify(mockFTPClient).sendCommand("quote SITE RDW"); > > verify(mockFTPClient).sendCommand("quote SITE RDW > > READTAPEFORMAT=V"); > > } > > ```
Many thanks for the suggestion. Yes it is to ensure we can send those FTP commands. I have adjusted as recommended. > On Dec. 3, 2018, 8:23 p.m., Szabolcs Vasas wrote: > > src/test/org/apache/sqoop/util/TestMainframeFTPClientUtils.java > > Lines 359-360 (patched) > > <https://reviews.apache.org/r/62523/diff/10/?file=2110996#file2110996line394> > > > > MainframeFTPClientUtils.getFTPConnection already invokes > > applyFtpCommands, so you don't have to invoke it again. Thanks again. It is included in the suggested unit test fix from the previous issue. - Chris ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62523/#review210979 ----------------------------------------------------------- On Dec. 10, 2018, 4:45 p.m., Chris Teoh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62523/ > ----------------------------------------------------------- > > (Updated Dec. 10, 2018, 4:45 p.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-3237 > https://issues.apache.org/jira/browse/SQOOP-3237 > > > Repository: sqoop-trunk > > > Description > ------- > > Added --ftpcmds command to allow comma separated list of FTP commands to send. > > > Diffs > ----- > > src/docs/user/import-mainframe.txt 3ecfb7e4 > src/java/org/apache/sqoop/SqoopOptions.java f06872f9 > src/java/org/apache/sqoop/mapreduce/mainframe/MainframeConfiguration.java > 9842daa6 > src/java/org/apache/sqoop/mapreduce/mainframe/MainframeImportJob.java > 90dc2ddd > src/java/org/apache/sqoop/tool/MainframeImportTool.java fbc8c3db > src/java/org/apache/sqoop/util/MainframeFTPClientUtils.java e7c48a6b > > src/test/org/apache/sqoop/mapreduce/mainframe/TestMainframeDatasetFTPRecordReader.java > 502e6333 > src/test/org/apache/sqoop/tool/TestMainframeImportTool.java 00e57bd0 > src/test/org/apache/sqoop/util/TestMainframeFTPClientUtils.java fc6e56d6 > > > Diff: https://reviews.apache.org/r/62523/diff/11/ > > > Testing > ------- > > Unit tests. > > > File Attachments > ---------------- > > SQOOP-3237-1.patch > > https://reviews.apache.org/media/uploaded/files/2017/09/26/56041556-e355-4372-83ab-1bcc01680201__SQOOP-3237-1.patch > > > Thanks, > > Chris Teoh > >
