overlord/snapstate: respect SnapMountDir in tests

[zyga]

Signed-off-by: Zygmunt Krynicki zygmunt.krynicki@canonical.com

[niemeyer]

Is there a real benefit in doing this? Why isn't that path hardcoded for these tests?

[zyga]

I want the tests to pass if that path is changed. This way we know that there's nothing hardcoded / hidden and buggy when the path is different.

[niemeyer]

We don't need to reassure that this path is not hardcoded on every single place we use snap.Info. If you want to make sure the path is right, let's please have one test that checks the path is right in the specific place you are concerned about, rather than duplicating that logic in hundreds of places.

[zyga]

Hmm, how would you suggest I do two things:

• patch SnapMountDir in a distribution
• run tests successfully

Without completing this patch set? I agree it's a bit annoying (there's just a few more small patches after this one and it's all done).

[niemeyer]

dirs.MockSnapMountDir("/snap")

E.g.:

https://github.com/snapcore/snapd/tree/master/release/release.go#L146

[Conan-Kudo]

@niemeyer I would disagree and suggest that having it not hardcoded anywhere and then being able to create mock tests where you try different paths and everything works is important. Snaps are supposed to not care about where they exist on the host filesystem, and this is a good way to prove it.

[Conan-Kudo]

👍

[niemeyer]

Per comment above, we don't want to sprinkle around Sprintfs on every use of snap.Info I think.

I'd suggest dirs.MockSnapMountDir("/snap"), similar to how we have MockOnClassic and MockReleaseInfo. See:

https://github.com/snapcore/snapd/tree/master/release/release.go#L146

(comment repeated here so it doesn't get buried)

[zyga]

I'll add mocking and reopen. Thanks :-)