GH-38310: [MATLAB] Create the test guideline document for MATLAB interface to arrow

[leihou6116]

Rationale for this change

This document is aimed at coaching people on how to write and run tests for the MATLAB interface. This document is helpful to reproduce and address test failures in GitHub Actions CI, and is also helpful to maintain the good quality of MATLAB interface.

What changes are included in this PR?

Created a markdown page under arrow/matlab/doc.

Are these changes tested?

No test needed.

Are there any user-facing changes?

No software change.

• Closes: [MATLAB] Create the test guideline document for MATLAB interface to arrow #38310

[github-actions]

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

In the case of PARQUET issues on JIRA the title also supports:

PARQUET-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

See also:

• Other pull requests
• Contribution Guidelines - How to contribute patches

[kevingurney]

Thank you @leihou6116 for the pull request! This is an excellent addition to the MATLAB interface documentation and should definitely help new contributors get familiar with our approach to testing! It's really great to see this being added!

I added a number of comments to the PR - but many of them are actually just minor suggestions for tweaking the wording of some sections.

Overall, I think this nicely addresses most of the core themes that are relevant for getting started with testing the MATLAB interface.

The only thing I think would be helpful to think about adding is a small section on "Testing Best Practices". This could be a simple bulleted list of things like:

1. Focus on testing one software "behavior" in each test case.
2. Test with both "expected" and "unexpected" inputs.
3. Add a comment at the beginning of each test case which describes what the test case is verifying.
4. Treat test code like any other code (i.e. use clear variable names, write helper functions, make use of abstraction, etc.)
5. Follow existing patterns when adding new test cases to an existing test class.
6. Use descriptive names for your test cases.
   .
   .
   .

[leihou6116]

Thank you @leihou6116 for the pull request! This is an excellent addition to the MATLAB interface documentation and should definitely help new contributors get familiar with our approach to testing! It's really great to see this being added!

I added a number of comments to the PR - but many of them are actually just minor suggestions for tweaking the wording of some sections.

Overall, I think this nicely addresses most of the core themes that are relevant for getting started with testing the MATLAB interface.

The only thing I think would be helpful to think about adding is a small section on "Testing Best Practices". This could be a simple bulleted list of things like:

1. Focus on testing one software "behavior" in each test case.
2. Test with both "expected" and "unexpected" inputs.
3. Add a comment at the beginning of each test case which describes what the test case is verifying.
4. Treat test code like any other code (i.e. use clear variable names, write helper functions, make use of abstraction, etc.)
5. Follow existing patterns when adding new test cases to an existing test class.
6. Use descriptive names for your test cases.
   .
   .
   .

Thank you Kevin for excellent review feedback. I'll address all and add the section of "Testing Best Practices".

[kevingurney]

Sorry, I meant to mention this before.

I noticed #31106 seems very similar to #38310. Once we merge this in, it probably makes sense to close out #31106, since it seems to essentially be a duplicate.

[kevingurney]

Thanks for working through all of the feedback I provided, @leihou6116! Overall, this is looking really nice!

I just made a few more suggestions based on the latest changes.

One high-level piece of feedback is that I think the ordering of the sections feels a bit confusing at the moment.

Perhaps, we could order them as follows:

1. Overview
2. Prerequisites
3. MATLAB Class-Based Unit Testing Framework
4. Running Tests Locally
5. Writing tests
6. General Guidelines
7. Test Organization
8. Continuous Integration (CI) Workflows
9. Code Coverage Goals
10. How to Check Code Coverage
11. Tips

[sgilmore10]

Thanks for working through all of the feedback I provided, @leihou6116! Overall, this is looking really nice!

I just made a few more suggestions based on the latest changes.

One high-level piece of feedback is that I think the ordering of the sections feels a bit confusing at the moment.

Perhaps, we could order them as follows:

1. Overview
2. Prerequisites
3. MATLAB Class-Based Unit Testing Framework
4. Running Tests Locally
5. Writing tests
6. General Guidelines
7. Test Organization
8. Continuous Integration (CI) Workflows
9. Code Coverage Goals
10. How to Check Code Coverage
11. Tips

My two cent would be to switch Running Tests Locally and MATLAB Class-Based Unit Testing Framework around. If you do that, you might even be able to delete MATLAB Class-Based Unit Testing Framework since the Writing tests section also discusses the framework to use when writing MATLAB tests.

[kevingurney]

This is looking fantastic now!

I only had a few very minor suggestions after reading this over again.

This looks like it is pretty much ready to be merged in.

Thank you!

[kevingurney]

This is a really fantastic contribution, @leihou6116!

Thanks so much for carefully working through all of the code review feedback.

This document will be super helpful going forward for both new and current contributors to the MATLAB interface! It's wonderful to see significant time and energy is being invested into building out the MATLAB documentation.

Thank you!!

[kevingurney]

+1

[conbench-apache-arrow]

After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit cb11e44.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.