Mach Command are Now Classless!

Firefox Tooling Announcements
#1

Now, just because they’re now classless, that doesn’t mean that they aren’t still classy :wink:.

More seriously, Alex Lopez has contributed the enormous improvement of promoting Mach command functions up to the module level.

Benefits:

  • Writing and maintaining Mach commands is now easier - there’s less complexity around object state and inheritance, and ergonomics has been improved due to the dramatically reduced boilerplate.
  • Writing integration tests for Mach commands is far simpler, because confusing tricks aren’t need to avoid failures caused by constructor side-effects.
  • Teams depending on Mach are more effective because they don’t need to spend as much effort understanding and maintaining associated code.

For example, defining Mach commands used to look like:

@CommandProvider
class Foo(MachCommandBase):
    @Command("foo", category="misc")
    @CommandArgument("--bar")
    def foo(self, bar):
        print(self.topsrcdir, bar)

Now, it looks like:

@Command("foo", category="misc")
@CommandArgument("--bar")
def foo(command_context, bar):
    print(command_context.topsrcdir, bar)

To write a test for such a command, it used to look like:

@mock.patch("sys.stdout", new_callable=io.StringIO)
def test_foo(mock_stdout):
    # The MachCommandBase constructor does a lot of work, including parsing the 
    # "mozconfig" file on-disk and processing a non-trivial amount of 
    # build-specific details. Since we don't want the external state of the 
    # on-disk mozconfig file affecting our test, we have to mock out the 
    # MachCommandBase constructor
    with mock.patch("mozbuild.base.MachCommandBase.__init__", return_value=None):
        command_obj = Foo(Mock())
        command_obj.topsrcdir = "<topsrcdir>"
        command_obj.foo(bar="baz")
        assert mock_stdout.getvalue() == "<topsrcdir> baz\n"

Now, it looks like:

@mock.patch("sys.stdout", new_callable=io.StringIO)
def test_foo(mock_stdout):
    foo(command_context=Mock(topsrcdir="<topsrcdir>"), bar="baz")
    assert mock_stdout.getvalue() == "<topsrcdir> baz\n"

:clap:!

Some fun facts:

  • Not only does Mach support this simpler behaviour, but all existing usages in-tree were ported to the new model, thereby maintaining code consistency.
  • The work to achieve this milestone was split into simple, mechanical (though still large) patches. This allowed us to land code in smaller chunks, while also keeping downstream merge conflicts relatively trivial to resolve.
    • For example, the final patch was essentially just a glorified dedent of existing mach_commands.py files.
  • Overall, this refactor affected roughly ~100 files and over ~13000 lines of code.
  • Alex has been fantastic to collaborate with, and I’m looking forward to seeing what projects he’ll tackle in the future.

Thanks Alex, and happy Mach-ing my fellow Mozillians :fox_face::sunglasses: