Refactor BFS tests to use gomega

[c0d1ngm0nk3y]

Fixes #234

Summary

Checklist

• I have viewed, signed, and submitted the Contributor License Agreement.
• I have linked issue(s) that this PR should close using keywords or the Github UI (See docs)
• I have added an integration test, if necessary.
• I have reviewed the styleguide for guidance on my code quality.
• I'm happy with the commit history on this PR (I have rebased/squashed as needed).

[c0d1ngm0nk3y]

@matejvasek As you are the original author of these tests, some feedback would be great if I understood them right. Note that it is not finished yet, e.g. I have to better understand them to name things.

[matejvasek]

@c0d1ngm0nk3y
I do not understand this part:

errOurs := fsutil.Walk(root, createWalkFn(&ourFiles))
Expect(errOurs).NotTo(HaveOccurred())

errTheirs := filepath.Walk(root, createWalkFn(&theirFiles))
Expect(errTheirs).NotTo(HaveOccurred())

How is it equivalent to:

if (ourFiles == nil) != (theirsFiles == nil) {
    t.Errorf("errors does not match, actual error: %#v, expected error: %#v", errOurs, errTheirs)
}

Maybe slightly better condition would be !errors.Is(errOurs, errTheirs) && !errors.Is(errTheirs, errOurs) or maybe errors.Unwrap(errOurs) != errors.Unwrap(errTheirs).

The point is to verify that our and stdlib walk func return same error, not that error is nil.
Speaking of which: would you mind adding a test similar to "non-existent root folder" but whit walkFn that propagates error?

[c0d1ngm0nk3y]

How is it equivalent to:

It is not at all. That is right. The point is that I didn't understand that part at all. It is exactly one test case, right? So either there are errors in both cases or not. It was used in 2 test cases and both do not return an error, so this was kind of pointless, imho.

Or did I miss something?

Speaking of which: would you mind adding a test similar to "non-existent root folder" but whit walkFn that propagates error?

When simplifying the code like this, I had also the feeling that this is missing. I will try to add.

[matejvasek]

It was used in 2 test cases and both do not return an error, so this was kind of pointless, imho.

Originally that code should have been common for multiple parametrized tests, some of them were supposed to return error. In particular "non-existent root folder" test should have used error propagating walk function but mistakenly I used error masking walk function.

[anthonydahanne]

Hello!
I suggest you use consts, when possible, extract the function from the before and explain the createTestDir function

Let me know if that makes sense!

[c0d1ngm0nk3y]

@anthonydahanne I think I have addressed your comments. Can we get this merged? Just let me know if I oversaw something.

[dmikusa]

Looks good to me. ASCII diagram for the win!

@c0d1ngm0nk3y can you update this one more time? Thanks

[anthonydahanne]

I rebased on my local fork; everything went fine, so I went ahead and did it here