-
Notifications
You must be signed in to change notification settings - Fork 64
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
Add tests to majority of the files. #92
Comments
I need to look at how closely is the GUI interlinked with the backend logic so we can add tests. |
Agreed, I've been thinking about how to do automated testing since there are way too many regressions with any major refactor/added feature and manually testing doesn't catch them all before pushing it to master...
Although now that I think about it layout-y stuff like the second fix would be easy enough to spot with the right testing, so maybe I was just deceiving myself ;) As far as the architecture goes, there's no separation ala MVC or anything like that, most widgets are self-contained and act on themselves by calling the appropiate methods. Some like Then there are many widgets that contain and manage other widgets. They're mostly containers or custom views. In fact the UI is mostly built like a tree structure where widgets contain other widgets and so on. For example, it the main view looks somewhat like this:
Events like key input are propagated through the stack until something accepts them and then handled they're, although this is mostly custom-coded in the respective on_key handlers. |
We have a lot of technical debt to pay, partly because there are little to no tests available. This makes development slow and cuber-some. There are currently 30 files with ~ 14K LOC in total, and yet there are no tests at all. We should start adding tests before implementing major new functionalities otherwise our technical debt will pile and this will severely impact future development.
We might also need to refactor a lot of code as some files > 1K LOC.
(Please consider this as an umbrella bug)
The text was updated successfully, but these errors were encountered: