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

Compile-time IL weaver to replace harmony and aspect injector for allure-xunit #369

Open
delatrie opened this issue Jul 3, 2023 · 0 comments
Assignees
Labels
task:improvement Change that improves some user experience but can't be considered a new feature

Comments

@delatrie
Copy link
Contributor

delatrie commented Jul 3, 2023

Starting from (most probably) 2.10.0-preview.1 we will be using harmony for the following tasks:

  1. Report parameters of xunit theories that couldn't be pre enumerated during the discovery phase. (part of Replace allure-xunit's custom attributes with runner reporter (fixes #344) #366)
  2. Selective run of tests in allure-xunit and allure-specflow (see Add support for selective test run with a testplan #372)

Since harmony is a runtime IL weaver, it has some limitations:

  1. generic methods can't be patched consistently
  2. inlined methods can't be patched at all
  3. breakpoints in patched methods don't work

Because of limitation 1 we can't use harmony to implement step attributes (step function may be a generic function).

Limitation 2 prevent harmony patches from working. That means the following stops working if the corresponding target method is inlined by the CLR:

  • Argument reporting by allure-xunit when the pre-enumeration is disabled (either explicitly or implicitly by xunit)
  • Selective run in allure-xunit
  • Selective run in allure-specflow

Limitation 3 is confusing. A workaround is to put a breakpoint somewhere in a nested call instead, but it isn't obvious to a user.

We also can't use AspectInjector instead of harmony because AspectInjector lacks generic API to select methods to patch (it only provided attribute-based API).

Eventually we're stuck with using two IL weavers in a single project.

Proposed solution

We may replace harmony and aspect injector with hand-written IL weaver that is based on Mono.Cecil (the library harmony and aspect injector are both based on). The weaver should just allow us to hook into xunit theories, fixtures, and step functions.

Motivation

The are two key motivations:

  1. To make allure-xunit behavior more straightforward and consistent with regard to debugging and report content.
  2. To reduce the number of dependencies.
@delatrie delatrie added the task:improvement Change that improves some user experience but can't be considered a new feature label Jul 3, 2023
@delatrie delatrie self-assigned this Jul 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
task:improvement Change that improves some user experience but can't be considered a new feature
Projects
None yet
Development

No branches or pull requests

1 participant