Return a wrapped fs.ErrNotExist if no spec directories are defined

[elezar]

See the discussion here: #308 (comment)

Here we return a wrapped fs.ErrNotExist error if there are no spec directories defined instead of a raw error that cannot be queried easily by the caller.

[elezar]

Thanks @klihub. While working on this, I was wondering whether we want to more strictly define (i.e. return an error from Configure) when no spec directories are defined? With that said, maybe this is a way for a user to instantiate an in-memory cache, but I would rather make that explicit rather than implicit.

[klihub]

Thanks @klihub. While working on this, I was wondering whether we want to more strictly define (i.e. return an error from Configure) when no spec directories are defined?

Yeah, at least my gut feeling is that it sounds like a reasonable idea.

Another option would be to update and document the cdi.WithSpecDirs option to ignore an empty director list and instead run with the defaults... since IFAICT using an explicit cdi.WithSpecDirs() is the only way how you can end up in this situation.

[elezar]

Another option would be to update and document the cdi.WithSpecDirs option to ignore an empty director list and instead run with the defaults... since IFAICT using an explicit cdi.WithSpecDirs() is the only way how you can end up in this situation.

I think this is a cleaner solution. Created #310 to do this.

[elezar]

rebased.

[fujitatomoya]

the fix looks good to me, but how about adding test case for this?

err := cache.RemoveSpec("something")
assert.True(t, errors.Is(err, fs.ErrNotExist))

[elezar]

I have added a basic test.

[elezar]

@klihub as also pointed out by the test, we do have a slight disconnect here. In the case where the spec dirs are empty, we return ErrNotExist, but in the case where os.Remove fails for the spec we are trying to remove due to ErrNotExist we return nil. Should we always return nil (or ErrNotExist) for consistency?

[klihub]

@elezar Can this still happen with #310 merged ?

[elezar]

It shouldn't be possible, no.

[elezar]

Does that mean we want to update the highestPrioritySpecDir implementation to not return an error?

[klihub]

I think it does not return an error. It returns a directory and its priority.