----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65530/#review197523 -----------------------------------------------------------
Ship it! Hi Dani, The fix works for me. Now, all we need is a committer to agree as well. ;) - Fero Szabo On Feb. 6, 2018, 12:15 p.m., daniel voros wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65530/ > ----------------------------------------------------------- > > (Updated Feb. 6, 2018, 12:15 p.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-3283 > https://issues.apache.org/jira/browse/SQOOP-3283 > > > Repository: sqoop-trunk > > > Description > ------- > > `org.apache.sqoop.manager.mysql.MySQLTestUtils#getCurrentUser()` executes > `whoami` in a subprocess if there's no USER environment variable (happened to > me while running tests from Docker). However, it waits for the Process > variable to become null, that never happens: > > ``` > // wait for whoami to exit. > while (p != null) { > try { > int ret = p.waitFor(); > if (0 != ret) { > LOG.error("whoami exited with error status " + ret); > // suppress original return value from this method. > return null; > } > } catch (InterruptedException ie) { > continue; // loop around. > } > } > ``` > > We could get rid of the while loop since `Process#waitFor()` blocks while it > completes. > > Note, that it's easy to workaround the issue by setting the USER environment > variable when running the tests. > > > Diffs > ----- > > src/test/org/apache/sqoop/manager/mysql/MySQLTestUtils.java 25dbe9d > > > Diff: https://reviews.apache.org/r/65530/diff/2/ > > > Testing > ------- > > Run `org.apache.sqoop.manager.mysql.MySQLCompatTest`. Failed with timout > without the patch. All 46 test cases pass in ~45 seconds with the patch in > place. > > > Thanks, > > daniel voros > >
