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

Add Wrathmark to Virtual Client #201

Merged
merged 42 commits into from
Nov 29, 2023
Merged

Conversation

rjmurillo
Copy link
Member

Adds Wrath-Othello from @ricomariani to VirtualClient. This is based on a series of articles published by Rico Part 1 and Part 2

Changes

  • Add VirtualClient.UnitTests internal visibility to VirtualClient.Actions assembly
  • Add WrathmarkMetricsParser to adapt Wrathmark stdout to Virtual Client Metric
  • Add WrathmarkLoadExecutor to run the Wrathmark console command

@rjmurillo rjmurillo changed the title [WIP] - Add Wrathmark to Virtual Client Add Wrathmark to Virtual Client Nov 10, 2023
@rjmurillo rjmurillo marked this pull request as ready for review November 10, 2023 21:21
Copy link
Contributor

@yangpanMS yangpanMS left a comment

Choose a reason for hiding this comment

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

Please add documents to /website/docs/workloads. Your markdown will automatically be hosted at https://microsoft.github.io/VirtualClient/docs/category/workloads/

@yangpanMS
Copy link
Contributor

Please also add one line in /website/docs/overview/overview.md with the basic info and license link

@ricomariani
Copy link

FWIW I released Wrathmark with a public domain license. No restrictions. The license is in the project.

@ricomariani
Copy link

Also you don't want to release while it is still pointing at the Vector128 version. That one is horribad. Use the std-arrays version. Peer directory. Same enlistment same change number.

@rjmurillo
Copy link
Member Author

Talked to @yangpanMS ; we're going to wait until #214 lands next week to merge. The update to .NET 8 for VC has some benefits and we'll use that as the baseline.

@rjmurillo
Copy link
Member Author

Failure appears unrelated to this change.

  Failed BufferTimeWaiterWaitsForExpectedAmountOfTime(Unix,X64) [36 ms]
  Error Message:
     Expected: True
  But was:  False

  Stack Trace:
     at VirtualClient.Dependencies.WaitExecutorTests.BufferTimeWaiterWaitsForExpectedAmountOfTime(PlatformID platform, Architecture architecture) in D:\a\VirtualClient\VirtualClient\src\VirtualClient\VirtualClient.Dependencies.UnitTests\WaitExecutorTests.cs:line 43

See Output Log L45903

CC @yangpanMS

@yangpanMS
Copy link
Contributor

Acknowledged, it's a flaky unit test. I will ignore that in another commit. For now you can rerun PR build and it mostly will pass

@rjmurillo
Copy link
Member Author

Acknowledged, it's a flaky unit test. I will ignore that in another commit. For now you can rerun PR build and it mostly will pass

Merged as of 342e1e3 and the build passed. Please review.

@rjmurillo
Copy link
Member Author

@yangpanMS Merged as of 1eded14 and the build passed. please review.

Copy link
Contributor

@yangpanMS yangpanMS left a comment

Choose a reason for hiding this comment

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

Sorry for the delayed review. Most of the comments are minor and are for consistency.

@rjmurillo rjmurillo closed this Nov 29, 2023
@rjmurillo rjmurillo reopened this Nov 29, 2023
@yangpanMS yangpanMS merged commit 96934a9 into microsoft:main Nov 29, 2023
3 checks passed
@rjmurillo rjmurillo deleted the Wrathmark branch November 29, 2023 18:52
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