Skip to content

Tests fail: Can't find git #440

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jun 7, 2015
Merged

Tests fail: Can't find git #440

merged 9 commits into from
Jun 7, 2015

Conversation

asbjornu
Copy link
Member

While attempting to add profiling to GitVersion in order to debug #437, I stumbled upon an issue running the following tests:

  1. PullRequestScenarios.CanCalculatePullRequestChanges
  2. PullRequestScenarios.CanCalculatePullRequestChangesFromRemoteRepo
  3. PullRequestScenarios.CanCalculatePullRequestChangesInheritingConfig
  4. GitPreparerTests.UpdatesExistingDynamicRepository
  5. GitHelperTests.CanDetermineTheVersionFromAFetchedFeature

They are all failing for the same reason: git can't be found. Here's the stack trace:

System.IO.FileNotFoundException : The executable file 'git' could not be found.
  ----> System.ComponentModel.Win32Exception : The system cannot find the file specified
   at GitVersion.Helpers.ProcessHelper.Start(ProcessStartInfo startInfo) in ProcessHelper.cs: line 40
   at GitVersion.Helpers.ProcessHelper.Run(Action`1 output, Action`1 errorOutput, TextReader input, String exe, String args, String workingDirectory, KeyValuePair`2[] environmentalVariables) in ProcessHelper.cs: line 79
   at GitTestExtensions.DumpGraph(IRepository repository) in GitTestExtensions.cs: line 18
   at EmptyRepositoryFixture.DumpGraph() in EmptyRepositoryFixture.cs: line 14
   at PullRequestScenarios.CanCalculatePullRequestChanges() in PullRequestScenarios.cs: line 19
--Win32Exception
   at System.Diagnostics.Process.StartWithCreateProcess(ProcessStartInfo startInfo)
   at System.Diagnostics.Process.Start(ProcessStartInfo startInfo)
   at GitVersion.Helpers.ProcessHelper.Start(ProcessStartInfo startInfo) in ProcessHelper.cs: line 26

So I created this branch to at least figure out that it was git it couldn't find, because as you can see from the stack trace, the Win32Exception that was originally thrown in isolate, is not very informative ("The system cannot find the file specified"; what file?).

Thus, this branch and PR does not fix the underlying problem, but at least makes the symptom of the problem discoverable. I hope we can discuss in this PR:

  1. Why the git executable can't be found
  2. Why it is needed
  3. Why CONTRIBUTING.md doesn't mention this
  4. What the recommended steps to fix the problem are
  5. What I can do to help

😃

MarkZuber and others added 5 commits April 30, 2015 14:46
Add NoFetch option to GitVersionTask.targets
… null (or empty) arguments throw ArgumentNullException.
…st instead of relying on Process.Start() to do it, since it doesn't say anything about which file it can't find (which makes debugging very hard).
…h for Win32Exception with NativeErrorCode equal to 2 translated to a more informative FileNotFoundException.
@JakeGinnivan
Copy link
Contributor

Thanks for the PR. It is used for the git log command to give a dump of the git graph. We could make the DumpGraph() method catch this exception and the write out a warning instead of the graph.

The reason for it is that when the test fails, having the commit graph logged out in the test output is really handy. And afaik libgit2sharp can't format logs like the git log command. (@nulltoken, can we replace https://github.com/ParticularLabs/GitVersion/blob/master/GitVersionCore.Tests/Helpers/GitTestExtensions.cs#L23 with libgit2sharp calls?)

We should add a note to contributing.

@asbjornu
Copy link
Member Author

asbjornu commented Jun 5, 2015

@JakeGinnivan I see. I've added a try/catch to DumpGraph() and now all tests are green. If this is an acceptable solution, there shouldn't be any need to add a note to CONTRIBUTING.md. I do agree that using libgit2sharp would be a better solution than a (possibly unavailable) command line git.exe, though, given that (as you write) libgit2sharp gives us the log adequately formatted.

If this PR looks good to you, it would be awesome to have it merged so I can rebase off of it to continue the profiling and testing of #456! 😄

@JakeGinnivan JakeGinnivan merged commit f0dbaf0 into GitTools:master Jun 7, 2015
@JakeGinnivan
Copy link
Contributor

All merged (including #456). Master should be in a good state now, if you could give it a run that would be awesome

@asbjornu asbjornu deleted the feature/improved-exception-handling branch June 8, 2015 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants