WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content

Conversation

@williambrode
Copy link
Contributor

Description:

Fixes #(issue)

Checklist:

  • Tests included.
  • Lint and formatter run with no errors.
  • Tests all pass.

Where applicable:

  • Public APIs match existing style.
  • Any breaking changes have a deprecation path or have been discussed.
  • Updated the vendor folder (using go mod vendor).

@williambrode
Copy link
Contributor Author

Perhaps a maintainer could suggest where to add a test for this? I could add it around the immediate function (Mount) but then I'd just be testing the specific implementation.

@Bluebugs
Copy link
Contributor

We currently have mostly unit test in the form of things that test how a function will provide a result depending on its input. That would be something you could easily add as a new file in internal/volume.

But these are not super useful as it only prove that things match your expectation, not that it work.

We do have some better/more useful end 2 end kind of tests with the CI that build full application using fyne-cross, but they are too broad and we don't get any code coverage out of them. I would love to see some more integration tests that actually show fyne-cross work and provide proper tests coverage. I think that is really further away at the moment.

Copy link
Member

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

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

Looks good, though it may imply the same should be done to the work dir in case it too is relative?

Regarding testing it may be that moving to helper methods which are easily tested is the way - a method named "Mount" would not necessarily be expected to do so much logic as it sounds pretty clear that it's mounting the paths. If the "cacheDirAbsOrDefault" helper existed then mount would essentially set up the object and create the dirs, a cleaner function overall I'd think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants