Skip to content

Conversation

@daviewales
Copy link
Contributor

WIP branch adding tests to Mount API for various backends.

@buhtz
Copy link
Member

buhtz commented Feb 17, 2025

Great. I am most of the days offline until next week. But i am on it

@daviewales daviewales force-pushed the mount-refactor-1-test-encfstools branch from 372913c to beaf7ec Compare February 18, 2025 00:40
@daviewales
Copy link
Contributor Author

Note that it's still WIP. I've added high-level tests for the 'local' backend, as it's the simplest. I'll work on adding tests for the remaining backends.

@daviewales daviewales force-pushed the mount-refactor-1-test-encfstools branch 2 times, most recently from 83e48f4 to ab70e8b Compare February 18, 2025 01:19
@buhtz buhtz added this to the 1.6.0 (2nd release from now) milestone Feb 22, 2025
@buhtz buhtz added the High label Mar 31, 2025
@buhtz buhtz added the Code Quality About code quality, refactoring, (unit) testing, linting, ... label Apr 2, 2025
@buhtz
Copy link
Member

buhtz commented Apr 25, 2025

  • I updated the branch to the latest dev.
  • I kind of recreated the new encode.py file so that its git history (from encfstools.py) is preserved. Because of a Microsoft GitHub bug you wont see this history at GitHub. But in your local branch you should see its history back to year 2012/13 with git log --follow encode.py (the --follow is important!).
  • Add encode.py to the list of files that will be checked for all linter rules.
  • Some minor adjustments.

I really like your tests. I can read and understand them very well. This is high quality.
I will dive a bit deeper. But for today I am finished. So you can work on your branch if you want to. I'll be back later.

@buhtz
Copy link
Member

buhtz commented Apr 26, 2025

I don't understand the purpose of MountWithLocalEncFS.test_unconfigured_mount(). What is the difference between "unconfigured" and "configured" in context of EncFS?

@buhtz buhtz added the Feedback needs user response, may be closed after timeout without a response label Apr 26, 2025
@daviewales
Copy link
Contributor Author

Unconfigured corresponds to the first run, before the BiT configuration or the EncFS directory is initialised.

Configured corresponds to later runs, where there is a preexisting configuration and EncFS directory.

@buhtz
Copy link
Member

buhtz commented Apr 27, 2025

Unconfigured corresponds to the first run, before the BiT configuration or the EncFS directory is initialised.

Configured corresponds to later runs, where there is a preexisting configuration and EncFS directory.

There is this out-commented line in test_mount.py

# unconfigured_mount = mount.Mount(cfg=cfg, tmp_mount=True)

To my unerstanding cfg here is configured for local-encfs. So how can it be unconfigured. I still don't get it, sorry.

I don't know that the desired test result could be for test_unconfigured_mount(). But I also don't understand all the mount behavior of BIT. Have not touched that part of the code base ever.

If you can be more specific with your questions and needs I might be able to help.

@daviewales
Copy link
Contributor Author

daviewales commented May 13, 2025

I've had a chance to look at this. 'Unconfigured' in this case refers to the EncFS configuration, rather than the BackInTime configuration. Yes, the BackInTime configuration is reused, but the EncFS mount has not yet been created. Perhaps I should rename it to test_uninitialised_mount()?

Edit:

Specifically, an initialised backup location will contain the file .encfs6.xml, while an uninitialised backup location will not.

@buhtz
Copy link
Member

buhtz commented May 13, 2025

"uninitialized" sounds good to me.

@buhtz
Copy link
Member

buhtz commented Jun 7, 2025

Hello David,
Can you please give me a short update about this PR?
Regards,
Christian

@daviewales
Copy link
Contributor Author

I haven't had much chance to work on the lately. The latest commit is work in progress, as the test I'm adding still needs some work to make it pass. In theory, it is supposed to create a new EncFS mount, and confirm that it was created.

@buhtz buhtz removed the Feedback needs user response, may be closed after timeout without a response label Jun 8, 2025
@daviewales daviewales force-pushed the mount-refactor-1-test-encfstools branch 3 times, most recently from 99f645a to a851399 Compare July 9, 2025 22:16
@buhtz
Copy link
Member

buhtz commented Jul 10, 2025

Hello David,
in the last days, I was quit active with refactoring. Have not done this if I had knew that you are working currently.

I am finished now and hope I have not interfered your work to much with conflicts.

Best,
Christian

@daviewales
Copy link
Contributor Author

Don't worry about me! Conflicts are expected with a slow moving branch like this. (And there weren't many.)

@daviewales daviewales force-pushed the mount-refactor-1-test-encfstools branch from 18ba540 to 556ddf4 Compare July 14, 2025 06:41
@daviewales
Copy link
Contributor Author

I'm getting a R1732 lint warning when creating a temporary directory, but I can't seem to disable the warning with noqa.

@buhtz
Copy link
Member

buhtz commented Jul 14, 2025

Which linter?
If pylint use "#pylint: disable=R1732"

Or follow the error and use "with"

Sorry. I am at holiday just using a touch device

@daviewales daviewales force-pushed the mount-refactor-1-test-encfstools branch from 556ddf4 to dbd204a Compare July 15, 2025 00:28
@buhtz
Copy link
Member

buhtz commented Oct 29, 2025

Hello David,
when the scheduling of my other (non BIT) tasks works as expected I would be kind of "free" in December. If you have no objections against it by then I would take over this PR (and #1897) about locale GoCrypt support.

Would this be OK for you?

It would help me a lot if you could somehow summarize what you have done so far. The thing is I never used EncFS (or GoCrypt). I am also not familiar with the mounting part of the code base. I never touched that part of BIT.

What do you think?

Regards,
Christian

PS: My idea is to have a first release candidate (1.6.0-rc1) offering local gocrypt support.

@daviewales
Copy link
Contributor Author

I'm happy for you to take this over if you get time.

Please feel free to ask any questions you have.

In summary, this branch is trying to add high level tests for EncFS. From memory, it currently tests basic local mount and unmount. It's tricky to test, because there are a few places where the EncFS mount control code triggers UI code, rather than cleanly separating the two. I've tried to mock these out, rather than rewriting the EncFS code, as I'm worried to do that without first having tests.

The other branch (#1897) is a rebase of an old branch by Germar which works for gocryptfs, but breaks EncFS. It makes a significant change to the parent MountControl class, which makes it incompatible with the existing implementation of EncFS. The change is that it separates initialisation from mounting, whereas the current EncFS code combines them, and adds initialisation within the mount code if necessary.

Previously, I wanted to update the existing EncFS code to work with this new MountControl parent class. However, I now wonder of it would be easier and safer just to adapt the gocryptfs code to work with the old MountControl parent class, as this would eliminate the need to modify EncFS code.

I found that there are a lot of complex interactions between GUI, cinfig, and mount code, which made it tedious to test manually, and easy to introduce regressions. Hence why I started this testing branch.

@buhtz
Copy link
Member

buhtz commented Dec 14, 2025

Hello David,
I try to keep it pragmatic. I like your approach with the tests. But as you stated yourself the codebase is hard to test in its current state/structure. The only notable modification I did to your PR is that I disable the EncFS mount tests via commenting them. I am also not able to solve it. And IMHO the costs for it are not worth the benefits.

The Debian Import Freeze for the upcoming Ubuntu LTS (release April) is in the end of January. I am quit sure that "local GoCrypt" won't make it into BIT until then. But I'll try. I really would like to hit the LTS release with GoCrypt support.

Thanks again for your efforts!

Regards,
Christian

@buhtz buhtz marked this pull request as ready for review December 14, 2025 10:05
@buhtz buhtz added PR: Waiting for review PR won't be merged until review and approval from a member of the maintenance team. PR: Merge after creative-break Merge after creative-break (min. 1 week) labels Dec 14, 2025
@buhtz buhtz merged commit e333e90 into bit-team:dev Dec 14, 2025
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Code Quality About code quality, refactoring, (unit) testing, linting, ... High PR: Merge after creative-break Merge after creative-break (min. 1 week) PR: Waiting for review PR won't be merged until review and approval from a member of the maintenance team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants