Update #uncommitted_changes? to handle missing Git

[jasonblalock]

• Fix for GitUtils.uncommitted_changes? throwing an error when called in
  an environment that does not have git installed
• Add tests for GitUtils.uncommitted_changes? scenariors
• Fixes Install generator fails in environment without Git #888

---

This change is 

[jasonblalock]

So for this I worked under the assumption that when Git wasn't installed there were definitely "uncommitted changes" and thus #uncommitted_changes? should return true. However I'm open to how you want to handle it.

[justin808]

see my question

thanks!

---

Reviewed 3 of 3 files at r1.
Review status: 1 of 3 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

lib/react_on_rails/git_utils.rb, line 8 at r1 (raw file):

      return false if ENV["COVERAGE"] == "true"
      status = `git status --porcelain`
      return false if !status.nil? && status.empty?

Is the status nil if git is not installed?

---

spec/react_on_rails/git_utils_spec.rb, line 7 at r1 (raw file):

module ReactOnRails
  RSpec.describe GitUtils do
    describe ".uncommitted_changes?" do

Is there a test for the case of git is not installed?

---

Comments from Reviewable

[jasonblalock]

Thanks, replied and updated.

---

Review status: 0 of 3 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

lib/react_on_rails/git_utils.rb, line 8 at r1 (raw file):

Previously, justin808 (Justin Gordon) wrote…

Is the status nil if git is not installed?

I've updated the code to explicitly look at the exit status of the command because I was seeing inconsistency on the return value of a command that was not found. So if exit status is 0 (successful) we can look at the result. Otherwise uncommitted_changes? returns true.

---

spec/react_on_rails/git_utils_spec.rb, line 7 at r1 (raw file):

Previously, justin808 (Justin Gordon) wrote…

Is there a test for the case of git is not installed?

Yes it's the third context. I've updated the description to be more explicit.

---

Comments from Reviewable

[jasonblalock]

@justin808 Looks like there was a ReadTimeout on one of the Travis builds, could somebody rebuild it?

[justin808]

If you don't have git installed, isn't this error message misleading? @jasonblalock

I'd say we get more "direct" and check if git is installed.

---

Reviewed 3 of 3 files at r2.
Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

lib/react_on_rails/git_utils.rb, line 12 at r2 (raw file):

      return false if $CHILD_STATUS.success? && status.empty?
      error = "You have uncommitted code. Please commit or stash your changes before continuing"
      message_handler.add_error(error)

If you don't have git installed, isn't this error message misleading? @jasonblalock

---

spec/react_on_rails/git_utils_spec.rb, line 47 at r2 (raw file):

          allow_any_instance_of(Process::Status).to receive(:success?).and_return(false)
          expect(message_handler).to receive(:add_error)
            .with("You have uncommitted code. Please commit or stash your changes before continuing")

this message is misleading if git is not installed

---

Comments from Reviewable

[jasonblalock]

So we already were checking if Git was installed via $CHILD_STATUS.success? so I updated the messaging to indicate if that were not the case.

---

Review status: 1 of 3 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

lib/react_on_rails/git_utils.rb, line 12 at r2 (raw file):

Previously, justin808 (Justin Gordon) wrote…

If you don't have git installed, isn't this error message misleading? @jasonblalock

Updated error messaging to indicate if Git is not installed

---

Comments from Reviewable

[jasonblalock]

Review status: 1 of 3 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

spec/react_on_rails/git_utils_spec.rb, line 47 at r2 (raw file):

Previously, justin808 (Justin Gordon) wrote…

this message is misleading if git is not installed

Fixed

---

Comments from Reviewable

[jasonblalock]

Updated with explicit messaging for git not being installed @justin808

[justin808]

Thanks @jasonblalock

---

Reviewed 2 of 2 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.

---

Comments from Reviewable