Assignment9+10: Tyler + Austin #87

twoody0 · 2024-12-11T02:47:14Z

…71-2024-Fall into Assignment9+10

…_UsingTpl_Success()

…ed it.

RunAsync_UsingTplWithCancellation_CatchAggregateExceptionWrapping() RunAsync_UsingTplWithCancellation_CatchAggregateExceptionWrappingTaskCanceledException()

…one is right.

…ledException

…tring>

LeoMarlow

Really nice work boys! I only found one issue thats sort of a non issue ? I think that your returning the wrong type of exemption with the cancelation. It seems to work tho anyway.
Anyway any extra detail is left in comments -L

Implement PingProcess' public Task RunTaskAsync(string hostNameOrAddress) ✔
First implement public void RunTaskAsync_Success() test method to test PingProcess.RunTaskAsync() using "localhost". ✔
Do NOT use async/await in this implementation. ✔
Implement PingProcess' async public Task RunAsync(string hostNameOrAddress) ✔
First implement the public void RunAsync_UsingTaskReturn_Success() test method to test PingProcess.RunAsync() using "localhost" without using async/await. ✔
Also implement the async public Task RunAsync_UsingTpl_Success() test method to test PingProcess.RunAsync() using "localhost" but this time DO using async/await. ✔
Add support for an optional cancellation token to the PingProcess.RunAsync() signature. ✔
Inside the PingProcess.RunAsync() invoke the token's ThrowIfCancellationRequested() method so an exception is thrown. ✔
Test that, when cancelled from the test method, the exception thrown is an AggregateException ✔/ ?
with a TaskCanceledException inner exception ✔/ ?
using the test methods RunAsync_UsingTplWithCancellation_CatchAggregateExceptionWrapping ✔
and RunAsync_UsingTplWithCancellation_CatchAggregateExceptionWrappingTaskCanceledException ✔ respectively.
Complete/fix AND test async public Task RunAsync(IEnumerable hostNameOrAddresses, CancellationToken cancellationToken = default) which executes ping for an array of hostNameOrAddresses (which can all be "localhost") in parallel, adding synchronization if needed. ✔
NOTE:
The order of the items in the stdOutput is irrelevant and expected to be intermingled.
StdOutput must have all the ping output returned (no lines can be missing) even though intermingled. ✔
Implement AND test public Task RunLongRunningAsync(ProcessStartInfo startInfo, Action<string?>? progressOutput, Action<string?>? progressError, CancellationToken token) using Task.Factory.StartNew() and invoking RunProcessInternal with a TaskCreation value of TaskCreationOptions.LongRunning and a TaskScheduler value of TaskScheduler.Current. Returning a Task is also okay.
NOTE: This method does NOT use Task.Run. ✔
Extra Credit
Test and implement PingProcess.RunAsync(System.IProgress progess) so that you can capture the output as it occurs rather than capturing the output only when the process finishes. ❌
Fundamentals
Place all shared project properties into a Directory.Build.Props file.✔
Place all shared project items into a Directory.Build.targets file.✔
Ensure nullable reference types is enabled ✔
Ensure that you turn on code analysis for all projects(EnableNETAnalyzers) ✔
Set LangVersion and the TargetFramework to the latest released versions available (preview versions optional) ✔
and enabled .NET analyzers for both projects ✔
For this assignment, consider using Assert.AreEqual() (the generic version) ✔
All of the above should be unit tested ✔
Choose simplicity over complexity ✔

Assignment.Tests/PingProcessTests.cs

Assignment/PingProcess.cs

LeoMarlow · 2024-12-11T20:12:35Z

Assignment.Tests/PingProcessTests.cs

+    }
+
+    [TestMethod]
+    [ExpectedException(typeof(AggregateException))]


Im not sure how you made this work. I had to flatten and rethrow as AggregateException here. When you cancel it should return a cancelationException

I thought that it was an aggregate exception until you flatten it and pull out the inner exception, but we will look more into it.

As far as I am aware, any errors caused within a task results in an aggregate exception

LeoMarlow · 2024-12-11T20:14:11Z

Assignment.Tests/PingProcessTests.cs

+        catch (AggregateException ex)
+        {
+            AggregateException flattened = ex.Flatten();
+            Assert.IsTrue(flattened.InnerExceptions.Any(e => e is TaskCanceledException),


I think this may be backwards CanceledExemption should be the default and Aggregate should be the one you flatten. It works like this but is kinda confusing

It is my understanding that you flatten the Aggregate to get the TaskCanceledException but I could be wrong. I will look more into it.

Here they catch an aggregate exception and flatten it. A flattened aggregate is still an aggregate exception. They then call the inner exceptions and check if it is a TaskCanceled Exception, which should be fine.

BDullanty

Implement PingProcess' public Task<PingResult> RunTaskAsync(string hostNameOrAddress) ✔
- First implement public void RunTaskAsync_Success() test method to test PingProcess.RunTaskAsync() using "localhost". ✔
- Do NOT use async/await in this implementation. ✔
Implement PingProcess' async public Task<PingResult> RunAsync(string hostNameOrAddress) ✔
- First implement the public void RunAsync_UsingTaskReturn_Success() test method to test PingProcess.RunAsync() using "localhost" without using async/await. ✔
- Also implement the async public Task RunAsync_UsingTpl_Success() test method to test PingProcess.RunAsync() using "localhost" but this time DO using async/await. ✔
Add support for an optional cancellation token to the PingProcess.RunAsync() signature. ✔

Inside the PingProcess.RunAsync() invoke the token's ThrowIfCancellationRequested() method so an exception is thrown. ✔
Test that, when cancelled from the test method, the exception thrown is an AggregateException ✔
- with a TaskCanceledException inner exception ✔
- using the test methods RunAsync_UsingTplWithCancellation_CatchAggregateExceptionWrapping ✔
- and RunAsync_UsingTplWithCancellation_CatchAggregateExceptionWrappingTaskCanceledException ✔ respectively.

Complete/fix AND test async public Task<PingResult> RunAsync(IEnumerable<string> hostNameOrAddresses, CancellationToken cancellationToken = default) which executes ping for an array of hostNameOrAddresses (which can all be "localhost") in parallel, adding synchronization if needed. ✔
NOTE:
- The order of the items in the stdOutput is irrelevant and expected to be intermingled.
- StdOutput must have all the ping output returned (no lines can be missing) even though intermingled. ❌
Implement AND test public Task<int> RunLongRunningAsync(ProcessStartInfo startInfo, Action<string?>? progressOutput, Action<string?>? progressError, CancellationToken token) using Task.Factory.StartNew() and invoking RunProcessInternal with a TaskCreation value of TaskCreationOptions.LongRunning and a TaskScheduler value of TaskScheduler.Current. Returning a Task<PingResult> is also okay.✔
NOTE: This method does NOT use Task.Run.✔

Extra Credit

Test and implement PingProcess.RunAsync(System.IProgress<T> progess) so that you can capture the output as it occurs rather than capturing the output only when the process finishes. ❌✔

Fundamentals

Place all shared project properties into a Directory.Build.Props file.✔
Place all shared project items into a Directory.Build.targets file.✔
Ensure nullable reference types is enabled ✔
Ensure that you turn on code analysis for all projects(EnableNETAnalyzers) ✔
Set LangVersion and the TargetFramework to the latest released versions available (preview versions optional) ✔
and enabled .NET analyzers for both projects ✔
For this assignment, consider using Assert.AreEqual<T>() (the generic version) ✔
All of the above should be unit tested ✔
Choose simplicity over complexity ✔

Good work :)

BDullanty · 2024-12-12T01:01:29Z

Assignment.Tests/PingProcessTests.cs

+    }
+
+    [TestMethod]
+    [ExpectedException(typeof(AggregateException))]


As far as I am aware, any errors caused within a task results in an aggregate exception

Assignment.Tests/PingProcessTests.cs

BDullanty · 2024-12-12T01:06:28Z

Assignment.Tests/PingProcessTests.cs

+        catch (AggregateException ex)
+        {
+            AggregateException flattened = ex.Flatten();
+            Assert.IsTrue(flattened.InnerExceptions.Any(e => e is TaskCanceledException),


Here they catch an aggregate exception and flatten it. A flattened aggregate is still an aggregate exception. They then call the inner exceptions and check if it is a TaskCanceled Exception, which should be fine.

Assignment.Tests/PingProcessTests.cs

BDullanty · 2024-12-12T01:13:03Z

Assignment/PingProcess.cs

+        void updateStdOutput(string? line) =>
+            (stringBuilder ??= new StringBuilder()).AppendLine(line);
+
+        Process process = RunProcessInternal(StartInfo, updateStdOutput, null, cancellationToken);


Perhaps add this within the Task.Run itself

What are your thoughts on why it should be inside?

BDullanty · 2024-12-12T01:15:29Z

Assignment.Tests/PingProcessTests.cs

+
+        // Act
+        Task task = Sut.RunAsync("localhost -c 4").ContinueWith(t => result = t.Result);
+        task.Wait();


Rather than continue with, perhaps using task.Wait() then just referring to task.Result may be easier to read

I will look into this. In your opinion would it just be easier or is it functionally better?

I changed that, I do feel like it's slightly more readable

BDullanty · 2024-12-12T01:21:01Z

Assignment.Tests/PingProcessTests.cs

+
+        // Assert
+        Assert.IsFalse(string.IsNullOrWhiteSpace(result.StdOutput));
+        Assert.AreEqual(0, result.ExitCode);


- StdOutput must have all the ping output returned (no lines can be missing) even though intermingled. ❌ Perhaps set a static number (like 9 * 4) based on Linux and check for that amount of lines

We changed it, thanks for the input!

…71-2024-Fall into Assignment9+10

github-actions · 2024-12-13T00:44:00Z

Summary


Generated on:	12/13/2024 - 00:43:58
Coverage date:	12/13/2024 - 00:43:56
Parser:	MultiReport (2x Cobertura)
Assemblies:	1
Classes:	2
Files:	1
Line coverage:	91.6% (121 of 132)
Covered lines:	121
Uncovered lines:	11
Coverable lines:	132
Total lines:	200
Branch coverage:	75% (30 of 40)
Covered branches:	30
Total branches:	40
Method coverage:	Feature is only available for sponsors
Tag:	1034_12307384732

Coverage

Assignment - 91.6%

Name	Line	Branch
Assignment	91.6%	75%
Assignment.PingProcess	91.6%	75%
Assignment.PingResult	100%

KennethWhite

This looks great! Looks like you knocked out all the assignment criteria, great code coverage as well! Also nice work on testing for the TaskCanceledException within the AggregateException without trying to rethrow.

twoody0 and others added 30 commits November 30, 2024 12:50

Moved everything up one directory

6667ed6

Added README

3833c30

Changed to get building

9af7c9a

Changed to get running

c32a993

Added editorcongif, added props

fab5f2d

Updated some methods

bb02178

Fixed warnings in StringExtensions

b8b919d

Merge branch 'Assignment9+10' of https://github.com/twoody0/EWU-CSCD3…

287ec90

…71-2024-Fall into Assignment9+10

fixed errors in PingProcessTests

d937f07

Suppressed warnings for WildCardPattern

a015772

Merge branch 'Assignment9+10' of https://github.com/twoody0/EWU-CSCD3…

f870e03

…71-2024-Fall into Assignment9+10

Made method static with a TODO to finish the method

fe215b0

Messed around making a test for RunTaskAsync

6d4ce91

Changed test to run static method. Might have to chnage back.

c0031a6

Removed parameter from the test that was not allowing us to build

d1eb845

Put warning suppressions on seperate lines

88debeb

Made some small changes to namespace etc.

5203a56

Implemented tests for RunAsync_UsingTaskReturn_Success() and RunAsync…

49dcea3

…_UsingTpl_Success()

Changed PingOutputLikeExpression whitespace

ac452e2

Wrote one test for RunAsync with cancelation token and then implement…

42ec286

…ed it.

Implemented these tests-

ee85d7c

RunAsync_UsingTplWithCancellation_CatchAggregateExceptionWrapping() RunAsync_UsingTplWithCancellation_CatchAggregateExceptionWrappingTaskCanceledException()

Messing around

191636e

Implemeted a couple tests for the LongRunnignAsync

2ed5f18

Implemented a coupe different versions of the method. Not sure which …

93b641d

…one is right.

Eddited some comments

76c6ec5

Updated test so that it throws an AggregateException with a TaskCance…

227d776

…ledException

Updated README

1aa70a3

Added another RunAsync method that has a return type of IEnumerable<s…

8f37781

…tring>

Added some tests, updated RunAsync

66f9cbf

Undid some changes

912239f

twoody0 added 15 commits December 11, 2024 11:08

Added more to the PingOutputLikeExpression

b1d312d

Changed PingOutputLikeExpression back.

6529c9c

More standard output testing

db28c08

More output testing

96a1df2

Changed AssertValidPingOutput back

3416074

Added additional using

bd88f87

Changed namespace

949519b

Changed namespace back

4064e53

Changed Run method

817ac37

Changed Run method back

119f199

Updated RunProcessInternal

eaedf1e

Updated Run method

92eb89b

Changed run methods back

09a070b

Changed some namespace

759caa0

Removed output expression checking

6d678b6

LeoMarlow reviewed Dec 11, 2024

View reviewed changes

AustinH5544 added 2 commits December 11, 2024 16:07

Removed redundant test

90bf5dd

Try again?

9a3118b

BDullanty reviewed Dec 12, 2024

View reviewed changes

twoody0 and others added 9 commits December 11, 2024 21:10

added white space

1daa026

Changed the parallel ping to test count lines

abc8f72

Corrected line count number for 4 pings.

15c4c80

Removed one of the two cancellationToken.ThrowIfCancellationRequested();

3c7f634

Merge branch 'Assignment9+10' of https://github.com/twoody0/EWU-CSCD3…

ca1d256

…71-2024-Fall into Assignment9+10

Added an AssertValidPingOutput

fa3c5d2

Removed unecessary method

1b6aaec

Removed commented out tests

891b3aa

Changed test so that it's more readable

bbfef8c

KennethWhite reviewed Dec 13, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Assignment9+10: Tyler + Austin #87

Assignment9+10: Tyler + Austin #87

twoody0 commented Dec 11, 2024

LeoMarlow left a comment

LeoMarlow Dec 11, 2024

AustinH5544 Dec 12, 2024

BDullanty Dec 12, 2024

LeoMarlow Dec 11, 2024

AustinH5544 Dec 12, 2024

BDullanty Dec 12, 2024

BDullanty left a comment

BDullanty Dec 12, 2024

BDullanty Dec 12, 2024

BDullanty Dec 12, 2024

AustinH5544 Dec 12, 2024

BDullanty Dec 12, 2024

AustinH5544 Dec 12, 2024

twoody0 Dec 13, 2024

BDullanty Dec 12, 2024

AustinH5544 Dec 12, 2024 •

edited

Loading

github-actions bot commented Dec 13, 2024

KennethWhite left a comment

Assignment9+10: Tyler + Austin #87

Are you sure you want to change the base?

Assignment9+10: Tyler + Austin #87

Conversation

twoody0 commented Dec 11, 2024

LeoMarlow left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BDullanty left a comment

Choose a reason for hiding this comment

Extra Credit

Fundamentals

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AustinH5544 Dec 12, 2024 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Dec 13, 2024

Summary

Coverage

KennethWhite left a comment

Choose a reason for hiding this comment

AustinH5544 Dec 12, 2024 •

edited

Loading