Proposal: Let’s Avoid Direct Commits to Distro RefApp

Development
1

A quick proposal. The distro refapp is used by almost everything in our dev workflow. If something breaks there, it immediately affects dev3, all frontend E2E tests, performance tests, OMOP ETL work, and basically anyone trying to develop with proxy or our nightly docker images. Even a small mistake can block merges or force teams to disable tests, and that hurts the reliability of the whole test suite.

To avoid that, here’s a simple workflow:

  • Make your change through a PR
  • Let the GitHub Actions run
    • Build and Validate Configuration will catch config or version issues
    • E2E tests will show any frontend impact
  • If everything looks good to you, you can merge the PR yourself if you are in a hurry. Or let someone review it if you want a second pair of eyes. I also enabled auto merge for this repo, so you can let it merge once checks pass and go make yourself a cup of tea while GitHub does the work.
  • Only commit directly if it has zero effect on the build.

This helps keep things stable for everyone. Open to feedback.

cc: @dev5

1 Like
2

Generally it sounds right. Also dev3 shouldn’t be updated unless E2E tests pass. The only issue I see is that Commits · openmrs/openmrs-distro-referenceapplication · GitHub have red cross next to almost all commits meaning their didn’t pass some tests… Are they legitimate issues or issues with tests?

3

It’s a true positive. The specific E2E test that fails is “Add, edit and cancel an appointment.” It fails because it cannot click on the “Add” button, and the same error occurs on dev3 as well.

4

There are a few issues here:

  1. At times, we actually want to merge changes that won’t pass E2E tests, e.g., the upgrade to core 2.8 is realistically a PR where we would expect breakage, but where we can’t fix thing until we have the update merged into the distribution (because the E2E tests can’t be updated to a working state until we’ve published the Docker images with the new version of the backend)
  2. There are times where the E2E tests fail, but those failures are completely unrelated to the change being proposed (this was more so the case when metadata was directly stored in the distro, but it’s kind of similar now). Relatedly, we’ve been observing that we’re seeing E2E failures on the esm repos after merging PRs where the E2E tests passed, which suggests that we really have an issue further up the chain.

1 is the thing that makes this unrealistic as a strongly enforced rule, but 2 is what makes it impractical for most current changes in the distro.

I would suggest, though, that one issue is making down-stream projects (like the OMOP ETL work and potentially the performance tests) depend on the dev builds when they should be relying instead on the QA builds—or for the OMOP ETL pipeline, the latest release, which are stable and should not have the same issues.

1 Like