[Experimental] Enable git hooks for code validation and setup test suite

[lambdaclan]

---

Title

Implement various qol features for improving code quality.

Pull Request Type

Experimental (discussion needed before merging upstream)

Related issue

N/A

Special note
I feel sad about having to even add this section but you never know who will be reading this so here it goes. All the "criticisms" mentioned below are made in good faith and in an attempt to point out the issues with the current setup so that we can improve the application we all use and care about. I made an effort to use constructive comments in a heartfelt, meaningful tone that does not imply criticism towards the maintainers and any contributors.

Description

Currently the project does not include an automated way to ensure that the code being committed and pushed upstream is validated against the measures we have in place such as ESLint and Prettier. Furthermore, although the project does have some checks in place the code overall is not in a valid state. It will take time and effort to make all files pass the checks so we need to make sure that it stays like that and that new additions do not change the status.

Linting, formatting and testing make more sense when run before committing/pushing code. By doing so we ensure no errors ever make it into the repository and maintain a consistent coding style for the multiple contributors working on the project regardless of the editors and IDEs they use. It will also speed up the PR process since the checks run by the CI should be green by default leaving more time for the maintainers to look into what actually matters the code changes without having to worry about styling and linting errors.

The check for either tool can be run manually but that can complicate things because the scripts will run the checks on the whole codebase rather than just the files that are staged in git. This can actually be disruptive, running a lint process on the whole project can be slow, linting/formatting results can be irrelevant to the work being done and the number of flagged files can be overwhelming putting people off from running the checks in the first place. We need a better way to run checks only on files that will be committed.

To continue with, the existing GitHub action workflows only check for linting errors but do not perform any formatting checks although the setup is already in place.

Beyond the topic of code styling checks and their automation the project currently does not have a test suite or any tests for that matter. I will not discuss the merits of software testing since I have already established (through the project's communication channels) that we are in agreement that a test suite for FreeTube is indeed desired. Having a testing framework in place will encourage authoring of tests for at least all new code and will hopefully spark an interest to go back and test the existing code as well.

Looking a bit further into the future, having tests that we can trust and rely on will allow us to better prepare for what I am assuming will inevitably happen down the road, the switch to Vue 3 and TypeScript. Those are quite big topics that will require a lot of effort and refactoring so any help we can get would be beneficial.

Feature list

This PR aims to tackle the issues mentioned above as follows:

• create a testing environment setup for unit and end to end (e2e) testing
• incorporate git hooks that automatically run ESLint and Prettier before each commit only on staged files and run all tests before each push
• adjust continuous integration to include format checks and test suite runner
• add additional checks that can improve code quality and style (import sorting order)
• initiate the discussion of how we can further improve the structure and organization of the project as a whole

Tools and frameworks

• Git hooks

  • simple-git-hooks was chosen over alternatives such as husky and pre-commit because is simple to setup and is lightweight.
  • simple-git-hooks is also the recommended hook management tool to be used alongside lint-staged, (for more info see here) which is what I used to run the validation checks only on files staged in git. lint-staged is also recommended by Prettier for automation purposes.
  • Both simple-git-hooks and lint-staged can be configured via config files (.simple-git-hooks.json and .lintstagedrc.json) which make adjustments fairly trivial.
  • simple-git-hooks automatically generate the git hooks after npm install through a post install script.
  • checks can bypassed via the no [--no-verify](https://git-scm.com/docs/githooks#_pre_commit) flag which is why we still need the checks to run in CI.

• Testing

  • Regarding information for testing Vue applications the official documentation provides all the necessary information. Note that the information for Vue 2 and 3 is largely the same. I went through the documentation, played with the various tools mentioned and picked what I felt made more sense.
  • Jest was the chosen test runner tool and unit testing for the following reasons:
    • it is already included in the dev dependencies of the project
    • it is the first recommendation by the Vue official documentation
    • it is "batteries included" with all necessary tools used in testing such as assertions and mocking
    • it works well and is the recommended testing tool to be used with the chosen component testing library (details mentioned below)
  • For component testing there are only two options. Vue Testing Library and Vue Test Utils. After trying out both, I settled with Vue Testing library for the following reasons:
    • it is newer and is built on top of Vue Test Utils, the official testing library for Vue which means most of the the features available in that library are still available plus a lot more (they hide some on purpose, please see here)
    • it re-exports query utilities and helpers from DOM Testing Library emulating querying the DOM in the same way the user would making tests more accessible and easier to write
    • it works hand in hand with Jest custom DOM matchers
    • their guiding principles are in line with how I feel testing should be done
    • it has nice, easy to understand documentation
  • For end to end testing the Vue official documentation mostly talks about web based applications with cross-browser testing which currently does not apply to FreeTube therefore the recommended tools were not considered. As FreeTube is Electron based the choice was easy as there are not many e2e testing frameworks for Electron apps. Spectron is the recommended framework by the Electron official documentation and although Mocha seems to be the recommended testing framework (https://github.com/electron-userland/spectron#usage) the documentation clearly states that "Spectron works with any testing framework" and can verify that it works just as good with Jest.

• General QOL additions

  • Although the git hooks will automatically run the checks for Prettier and ESLint, it will not automatically attempt to fix them (possible but decided not to), for this reason a new script prettier:local has been added which uses [pretty-quick] (https://github.com/azz/pretty-quick) to only format the changed or staged files since last commit files via Prettier. pretty-quick is also recommended by Prettier as a pre-commit mechanism. This script is meant to be run manually and possibly after the automated checks flag some files.
  • Re-worked the scripts so that tests can be run either via the test command or via the jest command.
  • Re-worked the scripts so that during tests only the relevant info is shown. For example e2e tests require packing before execution, the silent pack script quietly packs the app and then run the tests displaying only the test results in a clear manner.
  • Enabled headless testing for e2e tests that can be run during CI via xvfb. This was in accordance to Electron official documentation.

Screenshots and demos

Git hooks installation

Git hooks content

Pre commit demo 1

Pretty quick demo

Pre commit demo 2

Pre commit demo 3

Test suite demo

Pre push demo

Testing
Has this pull request been tested?
Please describe shortly how you tested it and whether there are any ramifications remaining.

This section will make more sense now that we will have a test suite 🎉 we can even turn down PRs (might be extreme for some) that do not include tests along with the changes added.

The overall functionality has been tested:

• ensure git hooks are automatically installed and setup properly
• ensure files are being validated via ESLint and Prettier before commits
• ensure files not being marked for validation do not trigger the hooks
• ensure tests are being run before pushes
• ensure test suite is in place for unit/component testing and e2e testing

Desktop:

• OS: Arch Linux
• FreeTube version: latest

Additional discussion points

• Do we need to adjust the rules of ESLint and Prettier in any way?
  • It might make sense to do any config changes before making the codebase pass all checks.
• How do we update the configuration for any of the checkers?
  • We need to stay up to date with the ruleset as things get added/modified all the time.
• Do we need to extend the checks to other files found in the repository?
  • Prettier supports a wide range of files and we could add checks for the GitHub workflow files (yaml)
  • ESLint can also be used for linting the workflows(https://www.npmjs.com/package/eslint-plugin-yml)
• Currently the build workflow is executed on pushes to master and development branches so provided the checks (formatting, linting and testing) are run before each PR we can safely assume that the build will succeed. Are those branches protected? If not should they be to avoid non validated builds?
  • The release and flatpak workflows are manually triggered, would it be better to create a new workflow that automatically validates, builds and releases the application?
• A script has been added to manually try to fix Prettier errors for changed/modified files. Do we need to add one for linting issues as well?
  • ESLint issues might be more complicated to fix meaning that user intervention might be needed so I avoided adding such a script but if we end up using eslint --fix then we will need a way to run that only on modified files somehow.
  • A potential solution could be to use the --cache flag of ESLint which only checks changed files and is off by default.

[efb4f5ff-1298-471a-8973-3d47447115dc]

Hi @lambdaclan Please update us about this. Is this ready for review?

[ChunkyProgrammer]

We switched to yarn for dependency insallation so you'll have to use npm run ci instead of npm ci

[PikachuEXE]

For me I would

• start small (less hooks, probably just running lint on commit at the beginning)
• use lefthook
  (but waiting for next release with fix on hook script and maybe a performance boost before I submit a draft PR)

[ChunkyProgrammer]

Suggested change

	"prettier": "prettier --write \"{src,_scripts}/**/*.{js,ts,vue}\"",
	"prettier": "prettier --check \"{src,_scripts}/**/*.{js,ts,vue}\"",

[ChunkyProgrammer]

Closing PR in favor of #1788