Unit test add odo-file-index.json to gitignore file #2441
Conversation
|
/retest |
|
@prietyc123 Needs rebase. Is this ready for review? |
openshift-ci-robot
added
the
needs-rebase
prietyc123
force-pushed
the
UnitTestAddOdoFileIndexJsonToGitignoreFile
branch
from
ceec79b
to
c1350f7
Compare
openshift-ci-robot
removed
the
needs-rebase
Rebase is done. Yes its ready for review. |
|
|
||
| func mockDirectoryInfo(create bool, contextDir string, fs filesystem.Filesystem) error { | ||
|
|
||
| if !create { |
There was a problem hiding this comment.
I'm a little confused. This condition will be satisfied when create is set to false. So when it's false, we're creating the file/directory and when it's true, we're not doing anything of the sort. This is by design, right?
Just trying to affirm my understanding. Correct me if I'm wrong.
There was a problem hiding this comment.
Correct @dharmit
In the test case where .gitignore directory does not exist, create will be set to true as we don't want that directory to exist.
There was a problem hiding this comment.
It feels counter-intuitive to me. In the scenario that you explained, why not have create set to false since we already have what we need? Since you have used the word "directory" twice, aren't we talking about .gitignore file here? I'm feeling lost here. 😞
There was a problem hiding this comment.
"directory" is the kind of context directory you can say where the component will get created. That directory we need to pass to the functions.
Again in second unit test https://github.com/openshift/odo/pull/2441/files#diff-0292f6b604e75b79cef052987afa7fb5R72 we should be having .gitignore file as already checked from https://github.com/openshift/odo/pull/2441/files#diff-0292f6b604e75b79cef052987afa7fb5R11
There was a problem hiding this comment.
In the earlier comment, you wrote:
In the test case where .gitignore directory does not exist
so I thought you're talking about a directory called .gitinore. Hence the question.
I'm still confused as to why we're creating a directory when create bool is false. 😕
There was a problem hiding this comment.
In the earlier comment, you wrote:
In the test case where .gitignore directory does not exist
so I thought you're talking about a directory called
.gitinore. Hence the question.I'm still confused as to why we're creating a directory when
createbool isfalse. 😕
Because whatever false directory we are creating will get created in RAM and once that unit test is complete it will delete the directory and everything.
As for https://github.com/openshift/odo/pull/2441/files#diff-0292f6b604e75b79cef052987afa7fb5R72 this unit test .gitignore file should be there to add odo-file-index into .gitignore.
Hence create bool is set to false, so that we can have .gitignore file there in the directory.
There was a problem hiding this comment.
so I thought you're talking about a directory called
.gitinore. Hence the question.
There are two different unit test, one for checking .gitignore exists or not. If not then create one. Second is to add odo-file-index.json into .gitignore.
If we can link these two unit test then create bool will be set to true. So is there any way we can do that as I am not aware of that. Please do suggest if there is any way to return .gitignore path from the first unit test I mentioned above. If not then we have have to create .gitignore directory as we are using fake filesystem with afero.
|
@girishramnani is probably the right person to review changes in fakefs pkg |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: girishramnani The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
|
/lgtm Looks good to me! |
openshift-ci-robot
added
the
lgtm
rm3l
added
the
estimated-size/L (20-40)
/kind test
What does does this PR do / why we need it:
Add unit test for odo-file-index.json into gitignore file
Which issue(s) this PR fixes:
Fixes #2404
How to test changes / Special notes to the reviewer:
make test