DESOSA 2022

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/

Figure: Top 10 Hotspot components in the app folder.

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.

Figure: New features frequently require a change in the long and important files, such as channel.js.

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/

Figure: Top 10 Hotspot components in the app/components folder.

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.

Figure: A bug, detected by SonarQube, indicating that a wrong operator being used: | instead of ||.

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


  1. Contribute Code to Mattermost https://developers.mattermost.com/contribute/getting-started/ Accessed: 20-Mar-22 ↩︎

  2. Contributing to Mattermost without a Ticket https://developers.mattermost.com/contribute/getting-started/contributions-without-ticket/ Accessed: 20-Mar-22 ↩︎

  3. Mattermost Contribution Checklist. https://developers.mattermost.com/contribute/getting-started/contribution-checklist/ Accessed: 19-Mar-22 ↩︎

  4. Mattermost code review. https://developers.mattermost.com/contribute/getting-started/code-review/ Accessed: 15-Mar-22 ↩︎

  5. Detox - GitHub. https://github.com/wix/Detox Accessed: 10-Mar-22 ↩︎

  6. Gray box testing. https://en.wikipedia.org/wiki/Gray_box_testing Accessed: 10-Mar-22 ↩︎

  7. Guide for Writing E2E Tests at Mattermost. https://developers.mattermost.com/contribute/mobile/e2e/guide-for-writing/ Accessed: 10-Mar-22 ↩︎

  8. 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 ↩︎

  9. Hotspot components. https://stackoverflow.com/questions/7686582/finding-most-changed-files-in-git Accessed: 19-Mar-22 ↩︎

  10. Mattermost Roadmap. https://mattermost.com/roadmap/ Accessed: 19-Mar-22 ↩︎

  11. SonarQube. https://www.sonarqube.org/ Accessed: 19-Mar-22 ↩︎

  12. Mattermost contribution guidelines and Code of Conduct. https://handbook.mattermost.com/contributors/contributors/guidelines/contribution-guidelines Accessed: 15-Mar-22 ↩︎

  13. Allow reply from individual notifications on Android | GitHub issue; https://github.com/mattermost/mattermost-mobile/pull/5494 Accessed: 20-Mar-22 ↩︎

  14. Edit Profile Screen without image picker | GitHub issue; https://github.com/mattermost/mattermost-mobile/pull/5849 Accessed: 20-Mar-22 ↩︎

  15. Saved posts screen and DB | GitHub issue. https://github.com/mattermost/mattermost-mobile/pull/6020 Accessed: 20-Mar-22 ↩︎

  16. Mattermost Documentation. https://docs.mattermost.com/ Accessed: 20-Mar-22 ↩︎

Mattermost Mobile
Authors
Wander Siemers
Cees Jol
Christie Bavelaar
Abel Van Steenweghen