Skip to content

Run plugin suite_finished handler before upload#82

Open
andmat900 wants to merge 4 commits intoeiffel-community:mainfrom
andmat900:20260330_fix_plugin_suite_finished_timing
Open

Run plugin suite_finished handler before upload#82
andmat900 wants to merge 4 commits intoeiffel-community:mainfrom
andmat900:20260330_fix_plugin_suite_finished_timing

Conversation

@andmat900
Copy link
Copy Markdown
Contributor

Applicable Issues

Fixes eiffel-community/etos#504

Description of the Change

Move the on_test_suite_finished plugin handler call to run before the Workspace context manager exits, which is where artifact upload happens. Previously the handler ran after artifacts were already uploaded, so plugins that needed to create additional artifacts upon suite completion had to handle upload themselves. Now plugins can simply write files to the artifact directory and they will be uploaded automatically.

Alternate Designs

Possible Drawbacks

Sign-off

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or

(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or

(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.

(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.

Signed-off-by: Andrei Matveyeu, andreimu@axis.com

Change-Id: I28dc8a64af90a5e93c4fa5070e787ef517f9193c
Signed-off-by: Andrei Matveyeu <andreimu@axis.com>
@andmat900 andmat900 requested a review from a team as a code owner March 30, 2026 10:42
@andmat900 andmat900 requested review from fredjn and t-persson March 30, 2026 10:42
self.logger.info("Figure out test outcome.")
outcome = self.outcome(result, executed, description, test_framework_exit_codes)
pprint(outcome)
self.logger.info("Call on_test_suite_finished plugin handlers.")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

debug

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

executed = False
description = str(exception)
raise
try:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need another level of nesting?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inner try/finally is needed so that on_test_suite_finished and outcome computation run inside the with Workspace block, before Workspace.__exit__ triggers artifact upload.

Without the inner try/finally, if tests raise an exception, the with block would exit immediately (uploading artifacts), and we'd never call the plugin handlers in time. The inner finally guarantees the plugin handlers run even on exception, but still before the workspace context manager cleans up and uploads.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then I would rather we create a method. I dont like the sight-line with three levels of nesting

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Change-Id: I3de8a51f19740f984841cdd0bf2a6ffce51e5b9b
Signed-off-by: Andrei Matveyeu <andreimu@axis.com>
Change-Id: Iff18d474ab3a5b1cb6a08a2765efdc4cd27adcc4
Signed-off-by: Andrei Matveyeu <andreimu@axis.com>
@andmat900 andmat900 requested a review from t-persson April 1, 2026 09:13
Change-Id: I22244003c771765710533b8b5ccbcb9705cea786
Signed-off-by: Andrei Matveyeu <andreimu@axis.com>
for plugin in self.plugins:
plugin.on_test_suite_finished(name, outcome)

def _run_and_finalize(self, workspace):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type-hints

Copy link
Copy Markdown
Collaborator

@t-persson t-persson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accidentally set approved when I had a comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ETR plugins: on_test_suite_finished handler runs too late

2 participants