protect workflow from errors in executive summary?

[cjyetman]

@jdhoffa should I wrap the executive summary stuff in a tryCatch() or something to protect against errors?

[AlexAxthelm]

I think that's a bad idea, since if would mean that errors in the ES would fail silently in our testing pipelines (which should be breaking if something is wrong on main)

[cjyetman]

maybe ES should have more robust testing on that end?

[AlexAxthelm]

🤷 Yeah. But I don't know enough about the content to be the one to set that up.

[jdhoffa]

1 - I agree with @AlexAxthelm and dont think anything should be done in this repo to changing how the errors of the dependent package are handled
2 - I agree with @cjyetman and pacta.executive.suymmary absolutely needs a way more robust testing framework, but don't have anybody with capacity that can/ wants to do this right now. @cjyetman if you're interested in giving it a go then by all means, but right now there are plenty of other actual content changes we need to make to ES on a pretty tight timeline

[cjyetman]

It's kinda frustrating to leave this repo in a state where its entire testing process breaks if a minor error is made somewhere else that I have little to no control over just to catch any problems with another repo, but I get that we don't have much capacity to improve the situation at the moment

[jdhoffa]

Yeah for sure. Although adding tests to pacta.executive.summary wouldn't have caught the issue either.

The problem is that a breaking change was introduced to pacta.executive.summary (the addition of a new/ necessary function parameter), that was not being called in workflow.transition.monitor.

Unit tests don't catch breaking changes to function APIs, that's gotta be noticed and communicated by the maintainer and/or reviewer, and PRs to broken dependencies should be opened simultaneously...