Mattermost Mobile — Quality and Evolution
Introduction
In this essay, we look at the quality and evolution of Mattermost Mobile, a team collaboration app for Android and iOS. We first look at how code is developed, tested and reviewed. Then we take a look at the current current source code to find so-called hotspots, pieces of code that change often, and assess the technical debt of this project. This should give us an idea how the architecture could still be improved.
Software quality process
Processes
Mattermost has set up processes to keep the code base well-organized. We divide these into three main processes: contribution, code review, and build processes. In this subsection, we focus on the contribution process.
The Mattermost contribution1 process can be divided into contributions with and without tickets. The former one targets issues that have already been reviewed by the Mattermost core team and have been flagged as “Help Wanted”. Here, open-source contributors can request to be assigned to the issue and commit their solution.
The latter one, contributions without tickets2, is used for minor corrections and improvements, as well as larger issues that are not yet recognized and for which a ticket should be created. A “Help Wanted” ticket ensures the changes are in line with the unified product vision of Mattermost. Opening such a ticket is done by starting a conversation with the core team in the feature idea forum, opening an issue in the GitHub repository, or chatting in the Contributors or Developers channels.
Once your contribution is ready, it must follow the contribution checklist3, this checklist provides a structure for submitting a pull request (PR) for your changes. After the PR is submitted it will go through the review4 process, where the code is examined by a member of the core team. This is discussed further in the PR section
Continuous integration process
Continuous integration (CI) automates the integration of code changes from multiple contributors into a single software project. Mattermost uses CircleCI for its CI pipeline. This pipeline contains multiple jobs, such as managing the various dependencies with Fastlane, and testing the app with Detox. The CI process separates the Android and iOS versions and deploys them in the app stores.
Testing
Mattermost implements some unit tests, but it is not part of the prescribed testing process. The project has implemented end-to-end testing using Detox. Detox5 is a gray box end-to-end testing and automation framework for mobile apps. Gray-box6 testing finds a middle ground between white-box testing and black-box testing. Black-box testing is used to test software without knowing the internal structure of the code. In white-box testing the tester uses their knowledge of the code to perform specific tests. Like black-box testing, gray-box testing is approached from a user perspective, but the knowledge of individual components is still used to find weak spots in the code. The Mattermost developer documentation describes the testing process for developers7. Issues are tracked on Jira. Before writing a test script the developer has to create a test case in Zephyr8. The Zephyr ID maps test cases to release testing specifications. This ID is used to measure coverage between manual and automated tests. If a test is failing due to a known issue, the developer should append the Jira issue key in the test description. The Detox tests are run using the Jest test runner. They are set up to run automatically on the CI server. This way, the developer will be notified when their changes break existing functionality.
Hotspot components
In this section, we look at the files that have frequently been committed to in the Mattermost Mobile repository, as well as anticipated hotspot components in the future.
Past
We analyzed the app folder and the app/components folder using the git log command9. We obtained the following output:
app/
The number on the left indicates the amount of commits that change the file. The most edited file in the app folder is the channel.js file. This may be because the file is very long: it has over 800 lines of code. Also, it is one of Mattermost’s most important features. In many PRs that we inspected, they often only edit a few lines of this file, for example, to make a new feature work.
After that, the most edited file is the mattermost.js file. This is the core file for Mattermost Mobile that launches the app. Although it is not long (~160 lines), it is a core file, which is presumably why it is changed often: new features often have some imports or configuration here. The few files after this file involve components, which we cover in the next paragraph.
app/components/
The post_list.tsx file and its previous names (post_list.js and post.js) combined is by far the most edited component in the repository. This component shows a list of messages (posts) and, optionally, a button to load more messages. The next few files are also related to posts. Thus, these files are the major hotspot components in the components folder.
Future
The public roadmap of Mattermost10 shows mainly core app features. These will change the core files similarly to how they have been changed before. When Mattermost puts more work into large features, like playbooks, these parts might become hotspots too.
Code quality
To analyze the quality of the repository, we scanned it using SonarQube. SonarQube is a tool to analyze code quality11. SonarQube is capable of analyzing JavaScript and TypeScript files, which the repository mainly consists of.
SonarQube has detected 18 bugs in the repository. Some bugs seem serious, but do not seem to break functionality. And because it doesn’t break functionality, the bug is not easily discovered. However, technical debt issues like these should be resolved in a repository that aims to be maintainable.
Further, many bugs are about useless assignments. First, four bugs involve unused values. Second, two bugs involve a useless expression: for example, Object.keys(...).length >= 0
. But wait… isn’t the number of keys in an object always larger or equal to zero…?
Lastly, there is a lack of unit testing: for example, the biggest hotspot React component in app/components has a test coverage of 0%. The biggest hotspot file in the app folder, actions/views/channel.js, also has a coverage of 0%. We see the lack of testing of hotspot components as an indication of low quality of the repository.
Quality culture
In the previous section, we looked at code quality in the Mattermost Mobile codebase. However, it is also interesting to see how Mattermost evaluates their code quality. Mattermost has set out some processes for contributions, such as its Contribution Guidelines12 and a code review process4. We looked at a selection of issues and PRs to see how these guidelines are implemented in practice. We used Mattermost’s public issue tracker and filtered on items mentioning the word architecture. We manually removed irrelevant issues/PRs and selected the latest ten items of each category.
Issues
Looking at the issues we selected - see Appendix A - we see that anyone can create issues and they are quickly assigned to the relevant team members. Most issues have significant back and forth between multiple commenters, such as engineers, engineering managers and quality assurance engineers. These are all positive indicators of quality. On the flip side, some issues are postponed seemingly indefinitely or are postponed to a future redesign and closed, but without linking to an issue for that future effort. That makes it hard to track if and when the requested architectural changes are implemented.
Pull Requests
For PRs, we again looked at ten recent PRs that mentioned architecture, and then manually selected ones that were architecturally relevant. We show an overview of this list in Appendix B. In these PRs, we see what the code review process looks like. The code review guidelines4 lay out rules for who should review PRs and when a PR can be merged. As can be seen in the selected PRs13 14 15, this process is followed rigorously and is also assisted by automation, for example by requiring issues with a certain label to be checked by QA staff. Senior developers actively comment on PRs we have reviewed.
Technical debt
We see quite some technical debt, mainly in component complexity. We regularly see files that are >800 lines long and functions that do not fit on a tall screen. While we believe there are no hard and fast rules for ideal size, we feel that this limits readability. The code also does not have a lot of comments explaining why something is done, which could make the codebase easier to understand.
However, the codebase in general is not hard to understand. This is aided by a good folder structure and clear naming. The project also uses standard React / React Native coding practices and components tend to be relatively modular, even though we would like them to be smaller.
To look more specifically at the hotspots identified in the section Code Quality, we used SonarQube to get an impression of technical debt. SonarQube reports a debt ratio of 0.2%, based on 713 code smells, and indicates that debt would take 15 days to solve. We, however, think that this estimation is highly optimistic, mostly due to the lack of unit testing as described before. SonarQube does not seem to take file size and complexity into account to calculate technical debt, and we think a score of 0.2% sounds too low. One hotspot, the channels view, is more than 800 lines long and has almost 40 lines of import statements. The post component, however, is a lot shorter and also a lot more readable.
Key quality attributes
Quality can be approached from multiple perspectives. In our first essay we looked at quality attributes that directly impact the user experience. One of the attributes we identified is reliability. Mattermost’s testing policy is meant to ensure this. Their E2E testing and code review policies are extensive. Even more importantly, the policies are followed, which contributes to the system’s reliability. In our assessment of code quality we found a number of bugs that currently do not impact functionality, but they may cause reliability issues in the future. Mattermost also has internal quality attributes that have a direct impact on the developer’s ability to effectively work with the code. Examples of these key qualities are simplicity and maintainability. During our search to assess quality and technical debt, we found that the source code for the mobile apps has a good folder structure and clear naming. This improves the simplicity of the project. Unfortunately we did find that the documentation, both on the website16 and in-code, is limited. The size of React modules and absence of unit tests also negatively impact the project’s maintabality.
Conclusion
The Mattermost Mobile team has some great policies in place to ensure the quality of the software. These processes work well for both the core team and other contributors, which is why they are actually followed. There is great communication and testing to limit technical debt, but there are no systems in place to reduce existing technical debt. Unresolved issues are also difficult to track. Finally, test coverage of hotspot components is lacking. So this is where the team could work to improve their workflow.
Appendix A: Overview of issues analyzed
Issue ID | Description |
---|---|
MM-41006 | Improve logging infrastructure by moving to one consolidated tool |
MM-39467 | Investigate making ModalController accessible on every route |
MM-37514 | Save playbooks in the store |
MM-38830 | Create dynamic child pipeline for cypress |
MM-36537 | Admin Advisor sends continuous emails about configuring support email in Cloud instances |
MM-32901 | FullScreenModal - when two or more opened, Esc keypress closes all |
MM-32207 | VPC Peering Architecture and Design |
MM-29822 | Draft a proposal for reworking data validation in the API/Service/Store layers |
MM-40045 | Q1: Technical Debt |
MM-31152 | Deep dive into the Message Export feature to ensure it is performant and robust at scale |
Appendix B: Overview of pull requests analyzed
Pull Request ID | Description |
---|---|
MM-39710 | Saved posts screen and DB |
5984 | Extract common observers to queries |
5937 | Fix & small refactor to the app entry logic and WS reconnection |
5718 | Gekidou teamsidebar |
5866 | copy directly from v1 |
MM-39712 | Edit Profile Screen without image picker |
5844 | [Gekidou] Post input |
5790 | [Gekidou] Adding logic to hide the list of teams on the teams sidebar when needed |
5499 | Integrate react-native-network-client |
MM-22933 & MM-36693 | Allow reply from individual notifications on Android |
Sources
-
Contribute Code to Mattermost https://developers.mattermost.com/contribute/getting-started/ Accessed: 20-Mar-22 ↩︎
-
Contributing to Mattermost without a Ticket https://developers.mattermost.com/contribute/getting-started/contributions-without-ticket/ Accessed: 20-Mar-22 ↩︎
-
Mattermost Contribution Checklist. https://developers.mattermost.com/contribute/getting-started/contribution-checklist/ Accessed: 19-Mar-22 ↩︎
-
Mattermost code review. https://developers.mattermost.com/contribute/getting-started/code-review/ Accessed: 15-Mar-22 ↩︎
-
Detox - GitHub. https://github.com/wix/Detox Accessed: 10-Mar-22 ↩︎
-
Gray box testing. https://en.wikipedia.org/wiki/Gray_box_testing Accessed: 10-Mar-22 ↩︎
-
Guide for Writing E2E Tests at Mattermost. https://developers.mattermost.com/contribute/mobile/e2e/guide-for-writing/ Accessed: 10-Mar-22 ↩︎
-
Zephyr Squad - Test Management for Jira. https://marketplace.atlassian.com/apps/1014681/zephyr-squad-test-management-for-jira?tab=overview&hosting=cloud Accessed: 10-Mar-22 ↩︎
-
Hotspot components. https://stackoverflow.com/questions/7686582/finding-most-changed-files-in-git Accessed: 19-Mar-22 ↩︎
-
Mattermost Roadmap. https://mattermost.com/roadmap/ Accessed: 19-Mar-22 ↩︎
-
SonarQube. https://www.sonarqube.org/ Accessed: 19-Mar-22 ↩︎
-
Mattermost contribution guidelines and Code of Conduct. https://handbook.mattermost.com/contributors/contributors/guidelines/contribution-guidelines Accessed: 15-Mar-22 ↩︎
-
Allow reply from individual notifications on Android | GitHub issue; https://github.com/mattermost/mattermost-mobile/pull/5494 Accessed: 20-Mar-22 ↩︎
-
Edit Profile Screen without image picker | GitHub issue; https://github.com/mattermost/mattermost-mobile/pull/5849 Accessed: 20-Mar-22 ↩︎
-
Saved posts screen and DB | GitHub issue. https://github.com/mattermost/mattermost-mobile/pull/6020 Accessed: 20-Mar-22 ↩︎
-
Mattermost Documentation. https://docs.mattermost.com/ Accessed: 20-Mar-22 ↩︎