Skip to main content

[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 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.


Back to the top