[
Date Prev][
Date Next][
Thread Prev][
Thread Next][
Date Index][
Thread Index]
[
List Home]
|
Re: [egit-dev] Is org.eclipse.egit.core--All-Tests broken in 'master'?
|
On 24 Oct 2009, at 11:32, Mykola Nikishov wrote:
Alex Blewitt <alex.blewitt@xxxxxxxxx> writes:
As I can see org.eclipse.egit.core--All-Tests is broken in
'master' for some time. Here are steps to reproduce:
So it looks like it's sensitive to the location in which it's
run. That doesn't sound like a good idea.
Yes, it's sensitive. But, for example, it's a RepositoryFinder's
nature,
just look its javadoc:
I wasn't suggesting that RepositoryFinder shouldn't do this; rather,
what I was saying, is that we shouldn't expect any random directory
location to (not) contain a .git parent repository.
Why don't we create a temporary directory in java.io.tmpdir and then
run any tests from there, rather than the current directory in which
we happen to be executing?
It wouldn't help in some cases. For example, I have a git repository
in
$HOME/.git and my temporary directory is $HOME/tmp/. BTW, changing
things in a parent directories IMHO considered as a bad practice ;-)
I'm not suggesting that we change parental directories; rather, I'm
saying that we should have a known, fresh location each time for
running the repository tests so that it doesn't rely on the assumption
that the current directory is (or isn't) already in a .git repository.
I don't see that we'll have control of where the user runs the tests,
even though we might be able to control it for the .launch.
And we wouldn't have that control in any case. We could try to use
some
virtual file system to isolate tests from the underlying filesystem or
use some sort of mocks/stubs for filesystem/RepositoryFinder/etc. I'm
out of ideas right now.
I think we should use a freshly created directory in the temp dir
(really, the only sensible default that one can assume) and then
instead allow that to be overridden for the specific cases (where TMP
is elsewhere). For example, on my system it's /var/folders/8r/
8rdnGb4cFae9PvyLCRDJw++++yU/-Tmp-/
The single solution, as I see it right now, is to have properly
configured unit tests with right preconditions asserted. If test
breaks
the reason of breakage should be clearly visible, not a cryptic
NPE. It's exactly what proposed patches do.
I agree with this approach as well. There's no reason why a test can't
fail in a more appropriate fashion; or even print out a warning that
it's skipping some tests owing to the location. But there's no reason
why we can't do both.
Alex
