Execute Action - Testing #742

kkeirstead · 2021-08-19T16:33:43Z

Adding independent tests for the Execute Action to show that it works before it's integrated in with triggers.

… Three basic test cases that in theory should work, but don't for reasons that might be local to my machine.

…ps due to the error saying -> the feature global using directive is currently in preview and unsupported. Planning to further flesh out testing and clean up code.

…s actually written to. Also ensures multiple arguments works properly. Still need to do cleanup and potentially add more tests.

…een before -> not sure what that means and if it's an issue, but made reference to broken pipes.

wiktork · 2021-08-19T17:34:35Z

src/Tests/Microsoft.Diagnostics.Monitoring.ExecuteActionApp/Program.cs

+    {
+        public static int Main(string[] args)
+        {
+            string testType = args[0];


Consider some argument validation to make it easier to figure out failures

What did you have in mind? In theory, since for each test we're manually setting the args, shouldn't we know that they'll all be valid?

For example:

Maybe the default case of the switch should throw an exception stating that the test type is not one of the known ones.

Maybe extract the remaining args into a new array that the test types can look instead of looking at the original args parameter. This allows us to change the ordering of the arguments passed on the command line without having to rewrite all of the test cases.

Each test type should valid that its args length is exactly what it should expect and throw an exception if it is different.

Directory.Build.props

src/Tests/Microsoft.Diagnostics.Monitoring.ExecuteActionApp/Program.cs

src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/ExecuteActionTests.cs

…g an additional test for IgnoreExitCode, small renames and adjustments, and the removal of the change to Directory.Build.Props

src/Tests/Microsoft.Diagnostics.Monitoring.ExecuteActionApp/Program.cs

src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/ExecuteActionTests.cs

kkeirstead · 2021-08-20T04:17:22Z

I noticed tests that return -1 as the exit code were failing unexpectedly (that didn't fail locally), claiming the exit code was 255. I changed the exit codes to 1 in the latest commit just to see if the tests will pass (and because fundamentally they still have the desired effect), but I'd be curious to know if anyone has any insight into this and whether it's an issue.

jander-msft · 2021-08-20T05:38:28Z

I noticed tests that return -1 as the exit code were failing unexpectedly (that didn't fail locally), claiming the exit code was 255. I changed the exit codes to 1 in the latest commit just to see if the tests will pass (and because fundamentally they still have the desired effect), but I'd be curious to know if anyone has any insight into this and whether it's an issue.

It might be that POSIX systems only allow for 8-bit exit codes from 0-255. So -1 becomes 255.

src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/ExecuteActionTests.cs

jander-msft · 2021-08-20T15:09:30Z

src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/ExecuteActionTests.cs

+            ExecuteOptions options = new();
+
+            options.Path = DotNetHost.HostExePath;
+            options.Arguments = GenerateArgumentsString(new string[] { "Sleep", (TokenTimeoutMs + DelayMs).ToString() }); ;


This might work most of the time. If it's not stable enough, maybe we can change this to write out a file (like the TextFileOutput scenario) and then sleep for some large amount of time. Back in the test, we start the process, wait for the file to appear, and then immediately cancel the token. This would provide more determinism so that if the host happens to be really slow that process startup doesn't exceed the token timeout.

* Initial collection rule options, validation, and unit tests. (#688) * Generate action and trigger options in schema. (#706) Generate action and trigger options in schema. Add display name descriptions to these options. Add defaults for the optional properties. * Create `Execute` collection rule action (#710) * Basic implementation and registration of actions and triggers (#736) Initial interfaces for collection rule triggers. Implement empty action and trigger implementations. Implement startup trigger with special interface denoting it is a startup trigger. Rename action provider and trigger provider to descriptor. Register action and trigger implementations with descriptors. * Execute Action - Testing (#742) * Update trigger interfaces with callback, start, and stop notions. (#760) Update trigger interfaces with callback, start, and stop notions. Refactor options providers into operations implementations. * Refactor host building into helper method (#769) * Refactor host building into helper method. * Triggers - Action List Executor (#768) * Implement collection rule pipeline, event counter trigger, and tests. (#825) Implement collection rule pipeline and event counter trigger. Add tests for collection rule pipeline with real and test triggers and actions. * Create CollectDump Collection Rule Action (#824) * Fix CollectionRulePipelineTests failure due to callback registration timing. (#854) * Collection rule service, logging, and some functional tests (#852) * Implement collection rule service for starting rules. Add logging throughout collection rule system. Add some functional tests for collection rule system. * Refactor common parameters into CollectionRuleContext. * Add safe cancellation. * Add collection rule completion timeouts. * Refactor logging event IDs into separate shareable class. * Adjust temporary directory creation and log cleanup failures. * Remove CollectionRulePipeline.StartAsync; replace with callback. Add safe awaiting for start tasks and run tasks. Add more logging and adjust some log levels. * Replace TestCommon dependency with Xunit.Assert (#874) Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: kkeirstead <[email protected]> Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: Wiktor Kopec <[email protected]> Co-authored-by: Patrick Fenelon <[email protected]>

kkeirstead added 5 commits August 17, 2021 20:16

Recreating the changes I accidentally deleted earlier today...whoops.…

d7c5e29

… Three basic test cases that in theory should work, but don't for reasons that might be local to my machine.

Three basic tests are now working. Had to change Directory.Builds.Pro…

1014578

…ps due to the error saying -> the feature global using directive is currently in preview and unsupported. Planning to further flesh out testing and clean up code.

Added in test that writes to a file and then ensures that the file wa…

1dc9038

…s actually written to. Also ensures multiple arguments works properly. Still need to do cleanup and potentially add more tests.

Cleaning up the tests.

3512c06

Minor tweaks. Noticed the presence of MSBuild_Logs, which I haven't s…

820a580

…een before -> not sure what that means and if it's an issue, but made reference to broken pipes.

kkeirstead requested a review from a team as a code owner August 19, 2021 16:33

wiktork reviewed Aug 19, 2021

View reviewed changes

jander-msft requested changes Aug 19, 2021

View reviewed changes

Merged with feature/Triggers; changes for Justin and Patrick includin…

7878c01

…g an additional test for IgnoreExitCode, small renames and adjustments, and the removal of the change to Directory.Build.Props

kkeirstead force-pushed the kkeirstead/triggers/ExecuteActionTesting branch from 28a4d26 to 7878c01 Compare August 19, 2021 23:38

jander-msft requested changes Aug 20, 2021

View reviewed changes

Small tweaks for Justin.

bd42c82

jander-msft approved these changes Aug 20, 2021

View reviewed changes

Small tweak for Justin.

21ad564

kkeirstead merged commit 9673941 into dotnet:feature/triggers Aug 20, 2021

kkeirstead deleted the kkeirstead/triggers/ExecuteActionTesting branch November 7, 2022 16:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Execute Action - Testing #742

Execute Action - Testing #742

kkeirstead commented Aug 19, 2021

wiktork Aug 19, 2021

kkeirstead Aug 19, 2021

jander-msft Aug 19, 2021

kkeirstead commented Aug 20, 2021

jander-msft commented Aug 20, 2021

jander-msft Aug 20, 2021

Execute Action - Testing #742

Execute Action - Testing #742

Conversation

kkeirstead commented Aug 19, 2021

wiktork Aug 19, 2021

Choose a reason for hiding this comment

kkeirstead Aug 19, 2021

Choose a reason for hiding this comment

jander-msft Aug 19, 2021

Choose a reason for hiding this comment

kkeirstead commented Aug 20, 2021

jander-msft commented Aug 20, 2021

jander-msft Aug 20, 2021

Choose a reason for hiding this comment