[SCM tests] Don't forget to change back to original cwd.

[SCM tests] Don't forget to change back to original cwd.

After removing the temporary SCM directory change back to the
original cwd. This will avoid errors on future os.cwd() calls.

BUG=412774
TEST=test-webkitpy --all

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=181894

[elecro, 2014-09-10 14:43:24]

pgal.u-szeged@partner.samsung.com changed reviewers:
+ abarth@chromium.org, dpranke@chromium.org

[Dirk Pranke, 2014-09-10 20:09:01]

This change seems safe, but unnecessary. I think most if not all of the
subsequent calls either use absolute paths or perform a cd themselves.

I'm not seeing any failures on either Mac OS 10.9 or Ubuntu Precise. Perhaps
something else is also going on that is causing the errors on Arch?

Or, could you point me at the specific code path where you think these changes
matter?

[elecro, 2014-09-11 08:23:54]

On 2014/09/10 20:09:01, Dirk Pranke wrote:
> This change seems safe, but unnecessary. I think most if not all of the
> subsequent calls either use absolute paths or perform a cd themselves.
> 
> I'm not seeing any failures on either Mac OS 10.9 or Ubuntu Precise. Perhaps
> something else is also going on that is causing the errors on Arch?
>
> Or, could you point me at the specific code path where you think these changes
> matter?

Ok, here are more details:

The SCMTestBase has a setUp method, which will set "original_cwd" instance
variable with
a 'self.fs.getcwd()' call an in the background that'll be an 'os.getcwd()' call.
Calling os.cwd
on a non existing directory will lead to an OSError (as shown in the bugreport).

In the two changed methods (_set_up_svn_repo, _set_up_svn_test_commits) chdirs
are performed
to change to the svn checkout path (This is OK). At the end of the method, the
checkout dir is
deleted (This is also OK). But we are currently in a non existing directory (So
any os.getcwd()
call will fail). 

After the svn repo setup, an actual checkout is performed, which due to that we
are in a non existing dir
the svn will report an error (I'll attach that error into the bug) and because
these steps are performed in
the setUp phase the tearDown is not called (which would change back to a correct
directory).
So it's svn checkout's faults mainly.

Additional note: I've also executed the './test-webkitpy --all' on Ubuntu
Precise and got the same result.

[Dirk Pranke, 2014-09-12 01:56:07]

On 2014/09/11 08:23:54, elecro wrote:
> On 2014/09/10 20:09:01, Dirk Pranke wrote:
> > This change seems safe, but unnecessary. I think most if not all of the
> > subsequent calls either use absolute paths or perform a cd themselves.
> > 
> > I'm not seeing any failures on either Mac OS 10.9 or Ubuntu Precise. Perhaps
> > something else is also going on that is causing the errors on Arch?
> >
> > Or, could you point me at the specific code path where you think these
changes
> > matter?
> 
> Ok, here are more details:
> 
> The SCMTestBase has a setUp method, which will set "original_cwd" instance
> variable with
> a 'self.fs.getcwd()' call an in the background that'll be an 'os.getcwd()'
call.
> Calling os.cwd
> on a non existing directory will lead to an OSError (as shown in the
bugreport).
> 
> In the two changed methods (_set_up_svn_repo, _set_up_svn_test_commits) chdirs
> are performed
> to change to the svn checkout path (This is OK). At the end of the method, the
> checkout dir is
> deleted (This is also OK). But we are currently in a non existing directory
(So
> any os.getcwd()
> call will fail). 
> 
> After the svn repo setup, an actual checkout is performed, which due to that
we
> are in a non existing dir
> the svn will report an error (I'll attach that error into the bug) and because
> these steps are performed in
> the setUp phase the tearDown is not called (which would change back to a
correct
> directory).
> So it's svn checkout's faults mainly.
> 
> Additional note: I've also executed the './test-webkitpy --all' on Ubuntu
> Precise and got the same result.

I wonder if you have a different version of svn than I do (1.6.17) or something.
I don't see this problem.

At any rate, I grant the theoretical possibility of the bug :).

LGTM.

[elecro, 2014-09-12 09:32:31]

On 2014/09/12 01:56:07, Dirk Pranke wrote:
[snip]
> I wonder if you have a different version of svn than I do (1.6.17) or
something.
> I don't see this problem.

On Arch I've got svn 1.8.10 and on Precise (fresh install) 1.8.8, so it could be
that
the error is related to the svn.


> At any rate, I grant the theoretical possibility of the bug :).
> 
> LGTM.

Thanks :)

[elecro, 2014-09-12 11:04:55]

The CQ bit was checked by pgal.u-szeged@partner.samsung.com

[commit-bot: I haz the power, 2014-09-12 11:05:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/558103003/1

[commit-bot: I haz the power, 2014-09-12 12:07:42]

Committed patchset #1 (id:1) as 181894