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

Functionality to attach files on execution level. #68

Conversation

PiotrNestor
Copy link

@PiotrNestor PiotrNestor commented Aug 30, 2024

@nvborisenko

Attach files on execution level. Env var is used as the mechanism to convey the attachment file name information.
Use an env var for file name patterns: rp_launch_logs

Example: Define the env var

rp_launch_logs = reports/*.log, *.json, env/default/*.properties

All files matched with the above pattern will be attached on the execution level of the report.

@PiotrNestor
Copy link
Author

@nvborisenko

How to change the agent version in the update?

@nvborisenko
Copy link
Member

I would see this kind of functionality on gauge side, like they do it with screenshots. Basically saying it would be good if client (test) can say to attach any binary as a artifacts to be saved later in the report.

Returning back to this PR - I have no "strong" opinion yet to decline it.

@jensakejohansson
Copy link

@nvborisenko I agree, we discussed, and also prefer, to programtically add attachments from the test client (Gauge) process. However, to add that solution in a nice way would mean quite alot of work in Gauge in a number of different repos (langauge runners, Gauge and probably proto repos). So, we opted for this less flexible, but mucher simpler, approach for now.

I hope you find it OK and that you can help @PiotrNestor to merge this PR if you don't see any obvious problems? I guess @PiotrNestor needs guidance on how to/where to update the version of the plugin so we don't build new binaries with same version number.

Thanks!

@nvborisenko
Copy link
Member

To update the version we use GitVersion, specifically:

  <Target Name="UpdatePluginManifest" AfterTargets="Build">
    <TokenReplace Path="$(OutputPath)\plugin.json" Token="$version$" Replacement="$(GitVersion_MajorMinorPatch)" />
  </Target>

which updates plugin version depending on git context.

Your proposed feature is interesting, it can be a part of the entire RP.NET ecosystem (NUnit, XUnit, Speflow and etc). I will research whether the implementation can be easily applied for gauge plugin. The big issue is in determining current working directory (in gauge world, I suppose, it is not a project directory).

@jensakejohansson
Copy link

Without checking any details I guess we don't have to update the version in our PR then, it will be handled by the pipeline/workflow?

We (@PiotrNestor and I) discussed the path/working directory situation. In our solution proposal we set the working directory, i.e. the starting point for the the rp_launch_logs search to the env. var. gauge_project_root. We found that to be a reasonable approach. If that variable for some reason should be empty (not set by Gauge) we fallback to Environment.CurrentDirectory. Have not verified this (I didn't write the code), but I suppose that would be the directory where the user were standing when executing the Gauge command / launched the process(es).

@nvborisenko
Copy link
Member

The version is handled automatically, don't worry.

I am thinking about a way how to move this functionality to shared repo. Usually the base file path is current working directory, but for gauge it seems we should respect GAUGE_PROJECT_ROOT env var.

@PiotrNestor
Copy link
Author

@nvborisenko
Are there any specific changes required for this PR/update?
What's the next step?

@nvborisenko
Copy link
Member

Move this functionality to ReportPortal.Shared package. It will be a plugin, which implements https://github.com/reportportal/client-dotnet/blob/develop/src/ReportPortal.Shared/Extensibility/IReportEventsObserver.cs

Each time, when reporting process wants to finish the launch, plugin reads files (depending on configuration) and sends them as launch attachments.

If you are able to help - please welcome!

@PiotrNestor
Copy link
Author

@nvborisenko
Can we get an updated release with this functionality of the report portal gauge agent?
What's the connection of 'ReportPortal.Shared' with this agent?

@nvborisenko
Copy link
Member

Can we get an updated release with this functionality of the report portal gauge agent?

I don't think so. Introduce new, then deprecate it, then introduce new with adjustments...

What's the connection of 'ReportPortal.Shared' with this agent?

This library is used by any .net agent and includes common functionality.

And this particular feature is a great candidate to be included for all agents, not only for gauge agent.

@jensakejohansson
Copy link

@nvborisenko

Ok, sound reasonable, but to be clear: Who adds it to the shared lib? Will you do that? Or do you expect us to do it?
We have little to no insight or understanding of how the shared lib works and has limited time / possiblites to test a solution proposal with other agents than Gauge.

@nvborisenko
Copy link
Member

I will do it. Please don't hesitate to ask status after couple of days.

At the same time is it ok if you can test it before releasing?

@jensakejohansson
Copy link

Great! Looking forward to it! We'll be glad to test that the Gauge-plugin works before releasing...

@jensakejohansson
Copy link

@nvborisenko Hi! What's the status on this?

@nvborisenko
Copy link
Member

@jensakejohansson almost done here reportportal/client-dotnet#148

@nvborisenko
Copy link
Member

Done, please help to review it: #70

@nvborisenko
Copy link
Member

@PiotrNestor @jensakejohansson this one #70 is released, can I close this PR if you don't mind?

@jensakejohansson
Copy link

Please do! I have not had time to test it/add it to my project yet, but I think @PiotrNestor did. Thanks for the help with this! 👍

@nvborisenko
Copy link
Member

Thanks for your help, feel free to contribute. It was a good collaboration.

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.

3 participants