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

Bugfix/fix reqnroll.verify parallelization #255

Merged
merged 4 commits into from
Sep 17, 2024

Conversation

ajeckmans
Copy link
Contributor

🤔 What's changed?

Changed the Reqnroll.Verify code to register a VerifySettings object per scenario, so that users can use this to execute the verify method correctly.

⚡️ What's your motivation?

Fixes #254

🏷️ What kind of change is this?

  • 📖 Documentation (improvements without changing code)
  • 🐛 Bug fix (non-breaking change which fixes a defect)
  • ⚡ New feature (non-breaking change which adds new behaviour)

♻️ Anything particular you want feedback on?

I see no way to verify that it does not work in parallel when not using the new settings object as it depends on the order executed and so sometimes will fail and sometimes not. (The first test that is ran most likely will always succeed.)

📋 Checklist:

  • I've changed the behaviour of the code
    • I have added/updated tests to cover my changes.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • Users should know about my change
    • I have added an entry to the "[vNext]" section of the CHANGELOG, linking to this pull

- Allow Reqnroll.Verify to run in parallel
by registering a per-scenario VerifySettings
- Added documentation on how to use Reqnroll.Verify.
@ajeckmans ajeckmans requested a review from gasparnagy September 9, 2024 09:51
Copy link
Contributor

@gasparnagy gasparnagy left a comment

Choose a reason for hiding this comment

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

This is good & thx for the tests too.
See my two smaller comments.

- Removed example for using verify in a single-threaded context as we would like users to not use that approach.
Copy link
Contributor

@gasparnagy gasparnagy left a comment

Choose a reason for hiding this comment

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

super, thx!

@gasparnagy gasparnagy merged commit 03ecd34 into main Sep 17, 2024
4 checks passed
@gasparnagy gasparnagy deleted the bugfix/fix-reqnroll.verify-parallelization branch September 17, 2024 12:30
gasparnagy added a commit that referenced this pull request Oct 14, 2024
* main:
  Bump version
  VS Code integration  proper doc title (#280)
  Fix: Reqnroll generates invalid code for rule backgrounds in Visual Basic (#284)
  Update nunit.md (#276)
  SolutionTests: Check if SDK version is installed and if not ignore the test (#266)
  Fix for #271 This PR modifies the Feature AST visitor to appropriately handle Rule Background steps. (#272)
  Bugfix/fix reqnroll.verify parallelization (#255)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants