Skip to content
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 automated pre-merge testing procedures to match OpenROAD's ambitions #23

Open
oharboe opened this issue Aug 7, 2019 · 3 comments
Open
Labels
documentation Improvements or additions to documentation enhancement New feature or request

Comments

@oharboe
Copy link
Contributor

oharboe commented Aug 7, 2019

OpenROAD has quite ambitious goals, so the automated pre-merge testing procedures need to be quite ambitious.

Once OpenROAD community gets going, there's easily going to be 100s of pending pull requests in flight across the projects and dozens of developers who depend on the stability of the master branch to work effectively.

It's trivial to manage a pending pull request: don't merge until it's completely ready, whereas it is an absolute nightmare, not to talk about catastrophic productivity loss, to manage unfinished work in the master branch that's interfering with development.

Another challenge for OpenROAD is that the automated tests will not scale easily. Each module should have a substantial amount of testing unique to that module, say up to 1 hour per pull request, but beyond that there needs to be quite substantial integration tests that tests the full flow. This full integration is probably going to have to be on the order of 24 hours.

Fortunately, these days this is a well studied problem from the world of DevOps, so solutions exist and are well known.

Rough outline of an example of how this can be achieved:

  1. Define responsibility for quality. If something breaks on master, then we shall assume that this happened despite the best efforts and intentions of everyone involved. The failure that crept into master is an example of a weakness in the automated procedures that should be addressed through more automated tests.
  2. Culture Walk the talk. Eat your own dogfood. Everyone create pull requests rather than push directly to master. Maintainers, especially, have a responsibility to show the community how to be good open source community citizens.
  3. Automated pull request tests: This can be achieved by hooking up e.g. Travis to build and test pull requests. A module has a budget of ca. 1 hour to do the most testing possible so that developers get feedback during their working day. https://travis-ci.org/
  4. Automated integration test and merge of pull requests. Nightly, all pull requests that have a thumbs up(some sort of manual vote + success from pull request build+test) are merged into a a pre-merge branch. The full set of integration tests are run on this pre-merge branch and if everything passes, the pre-merge branch is merged. If the pre-merge integration test failed, then the pre-merge branch is discarded and a notification is sent out. All pull requests gets a thumbs down vote, so they can be manually inspected before they are staged again.

Closing words:

I've had this sort of system in place for large complicated projects before and while it takes getting used to, it is absolutely fantastic once you get it going.

There are the technical benefits, but more importantly it greatly improves coordination and communication for pending pull requests and it creates a working environment where developers feel safe: they can't do anything wrong, the system will catch it.

Additionally, it's a great boost to overall productivity. Writing the code is a tiny part of a developers job. The majority of the work is in re-testing, coordination and debugging with the rest of the system.

Merging directly to master gives developers immediate gratification. Immediate gratification breeds addiction and terrible habbits. The prime example is that a developer merges something with master that someone else has to fix or complete: this abrasive behavior improves the productivity of the developer who merges with master branch at the cost of the productivity of everyone else.

The above is essentially a delayed reward system: only when everything is complete, does the developer get the reward of merging.

This approach maximizes the productivity of the team as a whole, rather than focusing on individual developers.

@tajayi
Copy link
Contributor

tajayi commented Aug 7, 2019

I agree 100% with everything you have said.
The next few weeks/months are dedicated to overhauling the builds, tests, and continous integration process. There are several plans in the works and prototypes being evaluated that are exactly along the lines of your suggestion. I'll be sure to point other developers on the team to your post.

Thanks

@oharboe
Copy link
Contributor Author

oharboe commented Aug 7, 2019

@tajayi It warms my heart to hear this :-) It's not easy to change the culture and infrastructure in an organization, but I'm convinced that OpenROAD stands no chance of reaching it's goals without a great community culture and quite an advanced, highly automated and robust automerge infrastructure

To implement this requires real money and is a lot like work, but it is absolutely imperative to have this in place to make effective use of contributes time. If this is not in place, nobody will dare touch code that's brittle and in dire need of being fixed.

TIP! Read up on git subtree when thinking about how to create this infrastructure.

What you can do with git subtree is that you can:

  • create a scaffolding git repository that has all the repositories in the build as folders within this single repository
  • a pull request to this scaffolding repository can then cover multiple upstream repositories
  • once a pull request to the scaffolding repository is automerged, git subtree then has the tools to automatically push and pull from the upstream git repositories. This applies to repositories that's owned by OpenROAD(like FasteRoute4-lefdef or alpha-release).
  • In the case of upstream repositories, like Yosis, the modification can rest in the scaffolding repository until it is accepted by the upstream yosis git repository. The upstream and scaffolding version of Yosis can then be merged in a git subtree pull.

The advantage of this approach are many:

  • The congitive load for the average developers is low. There's a single giant repository and they don't have to think about that there are multiple final repositories
  • It becomes easy to create scripts that build everything locally when it covers multiple repositories
  • It becomes trivial to keep multiple repositories in sync
  • The github build infrastructure for a single pull request will work even for pull requests that cover multiple repositories

@tajayi tajayi added documentation Improvements or additions to documentation enhancement New feature or request labels Aug 8, 2019
@oharboe
Copy link
Contributor Author

oharboe commented Aug 22, 2019

I've discovered that auto-merge must be a fairly common practice at github. There exists automation infrastructure as GitHub Apps.

The implementations I've found work as follows:

  1. Add a .yaml file to the git repository to configure automerge
  2. Enable the auto-merge

At this point, a pull request will automerge if configurable policies are met, typically automated testing passed and sufficient approval

Two implementations that I Googled, which appear very similar:

https://github.com/apps/probot-auto-merge
https://github.com/palantir/bulldozer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants