Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Isolates all swap-related functionality into a separate struct #105

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

bullgare
Copy link

@bullgare bullgare commented Mar 4, 2023

The idea is to remove cross-dependencies (and do not depend on a config singleton), make the code more modular and testable. It would lead to more predictable behaviour in the future and simplify further maintenance.

For this, all dependencies are injected to struct on instantiating.
I had to add spf13/afero to mock os.* calls, e.g. os.Open -> fs.Open.
Also added logger interface to unit-test all logged messages by each method.
The last update is introducing execCommander interface. By default (in a real app) it is falling back to exec.Command, for tests we replace it with a mock to make sure the commands running by each method.

The majority of added lines are actually unit tests for handler_swap.

@bullgare bullgare changed the title Isolates all swap-related functionality into a separate struct. Isolates all swap-related functionality into a separate struct Mar 4, 2023
file, err := os.Open("/proc/swaps")
func getSwapFileLocation(fs afero.Fs, defaultSwapFileLocation string) (string, error) {
filepath := "/proc/swaps"
file, err := fs.Open(filepath)
if err != nil {
return "", err
Copy link
Author

Choose a reason for hiding this comment

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

I've also noticed that this scenario does not fallback to a default swap file location.
Not sure if it was done intentionally.
It is easy to change with a defer if needed

@CryoByte33
Copy link
Owner

Sorry for the delay here, life has been crazy. Would you mind rebasing this on the new develop branch? I'm trying to make this a proper project now that there are contributors 🙂

@CryoByte33 CryoByte33 changed the base branch from main to develop November 29, 2023 04:48
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.

2 participants