Added tests for utils/git #2955

mansimarkaur · 2017-07-27T19:06:56Z

Have you followed the guidelines in our Contributing document?
Have you checked to ensure there aren't other open Pull Requests for the same change?
Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your changes? Here's an example.
Have you successfully run brew tests with your changes locally?

mistydemeo · 2017-07-27T22:41:26Z

Library/Homebrew/test/utils/git_spec.rb

Can you use system git, "init"? We only use backticks when we want the output as a string.

Using system to execute these git commands are failing the tests. git add and commit aren't being executed properly.

What's failing about them? It'd be good to fix that, rather than ignore the failure using backticks.

mistydemeo · 2017-07-27T22:52:23Z

Library/Homebrew/test/utils/git_spec.rb

Is this test title correct? before_commit isn't nil here.

I've corrected it now.

mistydemeo · 2017-07-27T22:53:13Z

Library/Homebrew/test/utils/git_spec.rb

Nitpick, but how about repo.join("file").to_s instead?

mistydemeo · 2017-07-27T22:59:35Z

Library/Homebrew/test/utils/git_spec.rb

Returns what?

Returns nil
Made the change

Don't see the change here?

its(:git_path) { is_expected.to be nil }

mistydemeo · 2017-07-27T23:00:56Z

Library/Homebrew/test/utils/git_spec.rb

We can't guarantee this path will always be this exact string - it could also be the Homebrew-installed git, the CLT-provided git, etc. For example, on my system:

expected: "/Applications/Xcode.app/Contents/Developer/usr/bin/git" got: "/usr/local/bin/git"

So, should I just test if the returned value inlcudes only "git"?

If we can stub it, let's do that - then it'll behave consistently even on systems that don't actually have git.

mistydemeo · 2017-07-27T23:02:35Z

Library/Homebrew/test/utils/git_spec.rb

You're testing the right behaviour, but this one surprises me! I didn't realize this was the case.

mistydemeo · 2017-07-27T23:09:29Z

Looks like we've got one failure:

  1) Utils::git_remote_exists when git is available returns true when git remote exists
     Failure/Error: expect(described_class.git_remote_exists(@repo)).to be_truthy
       expected: truthy value
            got: false
     # ./test/utils/git_spec.rb:151:in `block (4 levels) in <top (required)>'
     # ./test/spec_helper.rb:91:in `block (2 levels) in <top (required)>'
     # ./vendor/bundle/ruby/2.0.0/gems/rspec-wait-0.0.9/lib/rspec/wait.rb:46:in `block (2 levels) in <top (required)>'

mistydemeo · 2017-07-27T23:21:18Z

I get this additional failure locally:

  2) Utils::git_available? returns false if git --version command does not succeed
     Failure/Error: expect(described_class.git_available?).to be_falsey

       expected: falsey value
            got: true
     # ./test/utils/git_spec.rb:58:in `block (3 levels) in <top (required)>'
     # ./test/spec_helper.rb:91:in `block (2 levels) in <top (required)>'
     # ./vendor/bundle/ruby/2.0.0/gems/rspec-wait-0.0.9/lib/rspec/wait.rb:46:in `block (2 levels) in <top (required)>'

mistydemeo · 2017-07-31T17:56:46Z

A few failures on Linux:

  1) Utils::git_path when git is available returns path of git
     Failure/Error: expect(described_class.git_path).to eq("/Applications/Xcode.app/Contents/Developer/usr/bin/git")
       expected: "/Applications/Xcode.app/Contents/Developer/usr/bin/git"
            got: "/usr/bin/git"

As noted earlier, we can't count on the Xcode path to be there on every system.

  2) Utils::git_remote_exists when git is available returns true when git remote exists
     Failure/Error: expect(described_class.git_remote_exists("[email protected]:Homebrew/brew")).to be_truthy
       expected: truthy value
            got: false

Linuxbrew uses a different remote.

  3) Utils::git_available? returns true if git --version command succeeds
     Failure/Error: expect(described_class.git_available?).to be_truthy
       expected: truthy value
            got: false

Not sure what's going on here!

MikeMcQuaid

Made a bunch of style comments.

MikeMcQuaid · 2017-08-05T15:37:37Z

Library/Homebrew/test/utils/git_spec.rb

Favour using let over @ instance variables.

MikeMcQuaid · 2017-08-05T15:38:06Z

Library/Homebrew/test/utils/git_spec.rb

(repo/"file").write "blah"

I'll have to explicitly close the file as well if I write open and write separately. Why do you think (repo/"file").write "blah" is a better way?

MikeMcQuaid · 2017-08-05T15:38:22Z

Library/Homebrew/test/utils/git_spec.rb

(repo/"lib").mkpath

MikeMcQuaid · 2017-08-05T15:38:59Z

Library/Homebrew/test/utils/git_spec.rb

system "git", "add", "repo/#{file}" and same with all other calls

What are the reasons for preferring system "git", "add", "repo/#{file}" over system "#{git} add repo/#{file}" ?

MikeMcQuaid · 2017-08-05T15:39:33Z

Library/Homebrew/test/utils/git_spec.rb

Consider setting up the hash1 variable to avoid needing to [0..6] in the test

MikeMcQuaid · 2017-08-05T15:40:15Z

Library/Homebrew/test/utils/git_spec.rb

Use Pathname#/ rather than .join everywhere

MikeMcQuaid · 2017-08-05T15:40:42Z

Library/Homebrew/test/utils/git_spec.rb

Split this and other long lines onto multiple lines.

What are your views about adding a brew style violation for a line length greater than a certain number of characters? @mistydemeo @MikeMcQuaid
I have no clue how it can be done. Just a suggestion.

MikeMcQuaid · 2017-08-05T15:43:33Z

CC @reitermarkus for RSpec form check.

mansimarkaur · 2017-08-09T20:51:41Z

Library/Homebrew/test/utils/git_spec.rb

This test is behaving weird on my system. It passes sometimes and fails at other times.
Basically, git_available? is returning nil and false too at some times.

MikeMcQuaid

In general I think it's worth trying to minimise the usage of mocks/stubs here if possible. If not possible: no worries.

MikeMcQuaid · 2017-08-10T13:29:51Z

Library/Homebrew/test/utils/git_spec.rb

system git, "init"

MikeMcQuaid · 2017-08-10T13:30:38Z

Library/Homebrew/test/utils/git_spec.rb

(repo/file).write("blah"). That doesn't require an explicit close because it's handled automatically by Pathname#write.

I get the following error if I use Pathname#write :

Failure/Error: (repo/file).write("blah") RuntimeError: Will not overwrite repo/lib/blah.rb

MikeMcQuaid · 2017-08-10T13:30:45Z

Library/Homebrew/test/utils/git_spec.rb

(repo/file).write("blah"). That doesn't require an explicit close because it's handled automatically by Pathname#write.

I'd still say this form with a previous unlink is probably nicer but feel free to leave as-is and just change the case above.

MikeMcQuaid · 2017-08-10T13:32:46Z

Library/Homebrew/test/utils/git_spec.rb

Don't see the change here?

MikeMcQuaid · 2017-08-10T13:33:35Z

Library/Homebrew/test/utils/git_spec.rb

I think it may be worth allowing the git shim to be called. What fails if it is?

MikeMcQuaid · 2017-08-10T13:34:18Z

Library/Homebrew/test/utils/git_spec.rb

I think it may be worth allowing the git shim to be called. What fails if it is?

It returns different path values for different systems. The mocking attempts to avoid that.

@mansimarkaur I think we could just check for ends_with?("git") or similar. That would make this partially an integration test for the shim too, which would be great.

reitermarkus · 2017-08-10T17:30:59Z

Library/Homebrew/test/utils/git_spec.rb

its(:git_path) { is_expected.to be nil }

reitermarkus · 2017-08-10T17:32:22Z

Library/Homebrew/utils/git.rb

This is not the same as before.

The previous behaviour was wrong. If before_commit is not nil, it did not work correctly.

How exactly was it wrong?

If before_commit is nil, args become "--skip=1" but if before_commit is say, "0..3", then args should be "--skip=0" while previously args became just 0.

That's the correct behaviour. When no commit range is given, it should just skip the latest commit, i.e. --skip=1, otherwise, it should pass the first commit of the commit range to git log, so it will only show commits before that commit (before_commit).

So, then do you think the modifications made are correct? @reitermarkus

No, it should either be "--skip=1", or when there is a commit range, i. e. before_commit = "aabbcc...ddeeff", it should be just "aabbcc", not "--skip=aabbcc".

I understand it now. I've reverted the change.

reitermarkus · 2017-08-10T17:34:17Z

Library/Homebrew/test/utils/git_spec.rb

Don't use instance variables. Use let(:repo) { … }.

How do you suggest I use let with before(:all) and after(:all) blocks?

Why does it have to be before(:all)?

Realised I can do away with the before(:all) block. Hence, removed it. Thanks :)

reitermarkus · 2017-08-11T11:50:51Z

Library/Homebrew/test/utils/git_spec.rb

This isn't needed, since the test isn't OS-specific.

mansimarkaur · 2017-08-11T21:20:49Z

Library/Homebrew/test/utils/git_spec.rb

If I mock git_available? to return false and also mock the existence of CoreTap.instance.formula, running safe_system to install git raises an error since git is already installed on my system. What could be the workaround to mock a system that doesn't have brew installed git?
Thanks!
@mistydemeo @MikeMcQuaid @reitermarkus

expect(described_class).to receive(:safe_system).with(HOMEBREW_BREW_FILE, "install", "git").and_return(nil) should work to mock the safe_system call.

If git was installed successfully, safe_system would return true and not nil. How should we check for that since git will be pre-installed and an error will come up?

You can mock it to return true! It doesn't look like anything actually checks the return value of safe_system though - the error handling is in the system call it's wrapping, and since we're skipping that by mocking safe_system it should be okay.

MikeMcQuaid · 2017-08-12T11:48:01Z

Library/Homebrew/test/utils/git_spec.rb

(repo/file).write("blah") and skip the touch above.

This gives the following error:

Failure/Error: (repo/file).write("brew") RuntimeError: Will not overwrite repo/lib/blah.rb

Have you removed the touch above?

Yes. I have removed the touch. I still get the above mentioned error.

MikeMcQuaid

As a general comment: there's still a lot here that's relying on internal instance variables and state. It shouldn't ever be necessary to manually set an instance variable to hit full coverage on tests (and full line coverage rather than branch coverage is the goal). In general the more stuff is relying on internal implementation details in a class the more a simple refactoring of this class with require breaking and rewriting all of the tests. On the flip side, the more stubbing done inside the class the less we're testing the actual functionality. Ideally we want these to be essentially integration tests where we manipulate the inputs and outputs for the functions rather than the internals of the functions themselves.

MikeMcQuaid · 2017-08-12T11:49:11Z

Library/Homebrew/test/utils/git_spec.rb

I think this gets a bit too far into the implementation details. As @reitermarkus said: we shouldn't need to set instance variables in a test.

MikeMcQuaid · 2017-08-12T11:50:07Z

Library/Homebrew/test/utils/git_spec.rb

Is it possible instead to stub the lookup so it fails to find Git rather than overriding an instance variable?

Thanks for suggesting this, @MikeMcQuaid ! :)

mistydemeo

The new changes look good to me. I see there are still a couple of comments by @reitermarkus and @MikeMcQuaid which haven't been addressed.

reitermarkus · 2017-08-20T14:56:33Z

Library/Homebrew/test/utils/git_spec.rb

Move this into the it block, since it is only used once.

reitermarkus · 2017-08-20T14:56:47Z

Library/Homebrew/test/utils/git_spec.rb

Move this into the it block, since it is only used once.

reitermarkus · 2017-08-20T14:57:06Z

Library/Homebrew/test/utils/git_spec.rb

Move this into the it block, since it is only used once.

MikeMcQuaid · 2017-08-23T18:02:06Z

Library/Homebrew/test/utils/git_spec.rb

You don't need to touch here given you're immediately writing to it.

MikeMcQuaid · 2017-08-23T18:02:44Z

Library/Homebrew/test/utils/git_spec.rb

Could put this in one of these dirs to avoid the manual cleanup (and be consistent):

brew/Library/Homebrew/test/spec_helper.rb

Lines 110 to 129 in 2c43e06

FileUtils.rm_rf [

TEST_DIRECTORIES.map(&:children),

HOMEBREW_LINKED_KEGS,

HOMEBREW_PINNED_KEGS,

HOMEBREW_PREFIX/".git",

HOMEBREW_PREFIX/"bin",

HOMEBREW_PREFIX/"share",

HOMEBREW_PREFIX/"opt",

HOMEBREW_PREFIX/"Caskroom",

HOMEBREW_LIBRARY/"Taps/caskroom",

HOMEBREW_LIBRARY/"Taps/homebrew/homebrew-bar",

HOMEBREW_LIBRARY/"Taps/homebrew/homebrew-bundle",

HOMEBREW_LIBRARY/"Taps/homebrew/homebrew-foo",

HOMEBREW_LIBRARY/"Taps/homebrew/homebrew-services",

HOMEBREW_LIBRARY/"Taps/homebrew/homebrew-shallow",

HOMEBREW_REPOSITORY/".git",

CoreTap.instance.path/".git",

CoreTap.instance.alias_dir,

CoreTap.instance.path/"formula_renames.json",

]

MikeMcQuaid · 2017-08-23T18:03:04Z

Library/Homebrew/test/utils/git_spec.rb

Need to try and figure out a way to not set/get instance variables here. If need be: delete the relevant tests that need them set given we're almost at the end of GSoC. Ideally: just call other methods that end up calling these ones.

MikeMcQuaid · 2017-08-23T18:03:58Z

Library/Homebrew/test/utils/git_spec.rb

Instead of mktmpdir write to an already existing location that's deleted after tests: for example:

brew/Library/Homebrew/test/spec_helper.rb

Lines 110 to 129 in 2c43e06

FileUtils.rm_rf [

TEST_DIRECTORIES.map(&:children),

HOMEBREW_LINKED_KEGS,

HOMEBREW_PINNED_KEGS,

HOMEBREW_PREFIX/".git",

HOMEBREW_PREFIX/"bin",

HOMEBREW_PREFIX/"share",

HOMEBREW_PREFIX/"opt",

HOMEBREW_PREFIX/"Caskroom",

HOMEBREW_LIBRARY/"Taps/caskroom",

HOMEBREW_LIBRARY/"Taps/homebrew/homebrew-bar",

HOMEBREW_LIBRARY/"Taps/homebrew/homebrew-bundle",

HOMEBREW_LIBRARY/"Taps/homebrew/homebrew-foo",

HOMEBREW_LIBRARY/"Taps/homebrew/homebrew-services",

HOMEBREW_LIBRARY/"Taps/homebrew/homebrew-shallow",

HOMEBREW_REPOSITORY/".git",

CoreTap.instance.path/".git",

CoreTap.instance.alias_dir,

CoreTap.instance.path/"formula_renames.json",

]

MikeMcQuaid · 2017-08-23T18:04:08Z

Library/Homebrew/test/utils/git_spec.rb

Instead of mktmpdir write to an already existing location that's deleted after tests: for example:

brew/Library/Homebrew/test/spec_helper.rb

Lines 110 to 129 in 2c43e06

FileUtils.rm_rf [

TEST_DIRECTORIES.map(&:children),

HOMEBREW_LINKED_KEGS,

HOMEBREW_PINNED_KEGS,

HOMEBREW_PREFIX/".git",

HOMEBREW_PREFIX/"bin",

HOMEBREW_PREFIX/"share",

HOMEBREW_PREFIX/"opt",

HOMEBREW_PREFIX/"Caskroom",

HOMEBREW_LIBRARY/"Taps/caskroom",

HOMEBREW_LIBRARY/"Taps/homebrew/homebrew-bar",

HOMEBREW_LIBRARY/"Taps/homebrew/homebrew-bundle",

HOMEBREW_LIBRARY/"Taps/homebrew/homebrew-foo",

HOMEBREW_LIBRARY/"Taps/homebrew/homebrew-services",

HOMEBREW_LIBRARY/"Taps/homebrew/homebrew-shallow",

HOMEBREW_REPOSITORY/".git",

CoreTap.instance.path/".git",

CoreTap.instance.alias_dir,

CoreTap.instance.path/"formula_renames.json",

]

MikeMcQuaid · 2017-08-23T18:05:18Z

Library/Homebrew/test/utils/git_spec.rb

Write to an already existing location that's deleted after tests: for example:

brew/Library/Homebrew/test/spec_helper.rb

Lines 110 to 129 in 2c43e06

FileUtils.rm_rf [

TEST_DIRECTORIES.map(&:children),

HOMEBREW_LINKED_KEGS,

HOMEBREW_PINNED_KEGS,

HOMEBREW_PREFIX/".git",

HOMEBREW_PREFIX/"bin",

HOMEBREW_PREFIX/"share",

HOMEBREW_PREFIX/"opt",

HOMEBREW_PREFIX/"Caskroom",

HOMEBREW_LIBRARY/"Taps/caskroom",

HOMEBREW_LIBRARY/"Taps/homebrew/homebrew-bar",

HOMEBREW_LIBRARY/"Taps/homebrew/homebrew-bundle",

HOMEBREW_LIBRARY/"Taps/homebrew/homebrew-foo",

HOMEBREW_LIBRARY/"Taps/homebrew/homebrew-services",

HOMEBREW_LIBRARY/"Taps/homebrew/homebrew-shallow",

HOMEBREW_REPOSITORY/".git",

CoreTap.instance.path/".git",

CoreTap.instance.alias_dir,

CoreTap.instance.path/"formula_renames.json",

]

MikeMcQuaid · 2017-08-23T18:05:36Z

Library/Homebrew/test/utils/git_spec.rb

Use repo.cd do

MikeMcQuaid · 2017-08-23T18:06:25Z

Library/Homebrew/test/utils/git_spec.rb

Need to try and figure out a way to not set/get instance variables here. If need be: delete the relevant tests that need them set given we're almost at the end of GSoC. Ideally: just call other methods that end up calling these ones.

MikeMcQuaid · 2017-08-23T18:06:29Z

Library/Homebrew/test/utils/git_spec.rb

Need to try and figure out a way to not set/get instance variables here. If need be: delete the relevant tests that need them set given we're almost at the end of GSoC. Ideally: just call other methods that end up calling these ones.

MikeMcQuaid · 2017-08-23T18:06:48Z

Library/Homebrew/utils/git.rb

@reitermarkus This look correct to you?

No, it should still be the relative path. Might be that git show also supports absolute paths, but relative is definitely cleaner.

mansimarkaur · 2017-08-25T14:09:32Z

Please review, @MikeMcQuaid . Thanks!

mistydemeo · 2017-08-26T04:52:35Z

Looks good to me! Mind updating the commit history for merging?

…w/core is unavailable

Requested changes have been made

reitermarkus · 2017-08-29T13:41:36Z

Library/Homebrew/test/utils/git_spec.rb

Why are you using an instance variable here? Wasn't this only a workaround for before(:all)? let should work with before(:each).

MikeMcQuaid · 2017-09-02T14:53:36Z

Thanks again @mansimarkaur!

mistydemeo suggested changes Jul 27, 2017

View reviewed changes

MikeMcQuaid requested changes Aug 5, 2017

View reviewed changes

MikeMcQuaid requested a review from reitermarkus August 5, 2017 15:43

mansimarkaur force-pushed the git_tests branch from 0271131 to effa891 Compare August 9, 2017 20:49

mansimarkaur commented Aug 9, 2017

View reviewed changes

MikeMcQuaid requested changes Aug 10, 2017

View reviewed changes

reitermarkus requested changes Aug 10, 2017

View reviewed changes

reitermarkus reviewed Aug 11, 2017

View reviewed changes

mansimarkaur commented Aug 11, 2017

View reviewed changes

mansimarkaur force-pushed the git_tests branch from b23b8bb to c5a6774 Compare August 12, 2017 04:32

MikeMcQuaid reviewed Aug 12, 2017

View reviewed changes

MikeMcQuaid requested changes Aug 12, 2017

View reviewed changes

mansimarkaur force-pushed the git_tests branch from c5a6774 to 06de809 Compare August 15, 2017 19:09

mansimarkaur changed the title ~~[WIP] Added tests for utils/git~~ Added tests for utils/git Aug 16, 2017

mistydemeo approved these changes Aug 17, 2017

View reviewed changes

reitermarkus reviewed Aug 20, 2017

View reviewed changes

Library/Homebrew/test/utils/git_spec.rb Outdated

Copy link

Member

reitermarkus Aug 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this into the it block, since it is only used once.

reitermarkus reviewed Aug 20, 2017

View reviewed changes

Library/Homebrew/test/utils/git_spec.rb Outdated

Copy link

Member

reitermarkus Aug 20, 2017 •

edited

Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this into the it block, since it is only used once.

reitermarkus reviewed Aug 20, 2017

View reviewed changes

Library/Homebrew/test/utils/git_spec.rb Outdated

Copy link

Member

reitermarkus Aug 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this into the it block, since it is only used once.

MikeMcQuaid previously requested changes Aug 23, 2017

View reviewed changes

mansimarkaur force-pushed the git_tests branch from 65d7b48 to 9224444 Compare August 25, 2017 11:44

mistydemeo approved these changes Aug 26, 2017

View reviewed changes

mansimarkaur force-pushed the git_tests branch 2 times, most recently from 4e07853 to df3be75 Compare August 26, 2017 19:48

Added tests for last_revision_commit_of_file

d91036e

mansimarkaur added 6 commits August 27, 2017 01:23

Added tests for git_available?

657b6ef

Added tests for Utils

660617a

Used system command to run git commands

4f0f55a

Removed shutup

abe78eb

Added tests for ensure_git_installed when git is available or homebre…

372a422

…w/core is unavailable

Added clear git version cache method

c695dff

MikeMcQuaid approved these changes Aug 29, 2017

View reviewed changes

reitermarkus reviewed Aug 29, 2017

View reviewed changes

mansimarkaur force-pushed the git_tests branch from df3be75 to 784250d Compare August 29, 2017 17:17

Used already existing clear_git_available_cache to remove @git

784250d

reitermarkus approved these changes Aug 30, 2017

View reviewed changes

MikeMcQuaid merged commit 3bb9871 into Homebrew:master Sep 2, 2017

Homebrew locked and limited conversation to collaborators May 4, 2018

	FileUtils.rm_rf [
	TEST_DIRECTORIES.map(&:children),
	HOMEBREW_LINKED_KEGS,
	HOMEBREW_PINNED_KEGS,
	HOMEBREW_PREFIX/".git",
	HOMEBREW_PREFIX/"bin",
	HOMEBREW_PREFIX/"share",
	HOMEBREW_PREFIX/"opt",
	HOMEBREW_PREFIX/"Caskroom",
	HOMEBREW_LIBRARY/"Taps/caskroom",
	HOMEBREW_LIBRARY/"Taps/homebrew/homebrew-bar",
	HOMEBREW_LIBRARY/"Taps/homebrew/homebrew-bundle",
	HOMEBREW_LIBRARY/"Taps/homebrew/homebrew-foo",
	HOMEBREW_LIBRARY/"Taps/homebrew/homebrew-services",
	HOMEBREW_LIBRARY/"Taps/homebrew/homebrew-shallow",
	HOMEBREW_REPOSITORY/".git",
	CoreTap.instance.path/".git",
	CoreTap.instance.alias_dir,
	CoreTap.instance.path/"formula_renames.json",
	]

Uh oh!

Added tests for utils/git #2955

Added tests for utils/git #2955

Uh oh!

Conversation

mansimarkaur commented Jul 27, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mistydemeo commented Jul 27, 2017

Uh oh!

mistydemeo commented Jul 27, 2017

Uh oh!

mistydemeo commented Jul 31, 2017

Uh oh!

MikeMcQuaid left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mansimarkaur Aug 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mansimarkaur Aug 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MikeMcQuaid commented Aug 5, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MikeMcQuaid left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

mansimarkaur Aug 9, 2017 •

edited

Loading

mansimarkaur Aug 9, 2017 •

edited

Loading

mansimarkaur Aug 11, 2017 •

edited

Loading