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

Adding unit tests for fault request ordering #4409

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

mye956
Copy link
Contributor

@mye956 mye956 commented Oct 23, 2024

Summary

This PR will add additional unit testing for the network fault injection handlers, specifically the ordering of when each request finishes if they're modifying the same resource (e.g. the same network namespace/network interface).

The ordering/workflow is the following:

  • Make an asynchronous start fault request that takes some time to finish and starts being handled
  • Make an asynchronous stop fault request that does not take any time to finish and waits until the start fault finishes being handled
  • Both requests should be successful

Implementation details

  • makeAsyncRequest() : Helper function that is called upon in a goroutine to make mock HTTP requests
  • New tests cases added in TestNetworkFaultRequestOrdering(): The test cases will make two sequential asynchronous HTTP request (1 start and then 1 stop fault request). We will be mocking the first request (start fault) to be taking some time to finish while the second request (stop fault) should wait until the first request finishes before it gets executed.

Testing

New test cases that were introduced:

  • Make 1 asynchronous start black hole port fault request and then 1 asynchronous stop black hole port request
  • Make 1 asynchronous start latency fault request and then 1 asynchronous stop latency request
  • Make 1 asynchronous start packet loss fault request and then 1 asynchronous stop packet loss request

New tests cover the changes: yes

Description for the changelog

Feature: Adding network fault request ordering unit test cases

Additional Information

Does this PR include breaking model changes? If so, Have you added transformation functions?

Does this PR include the addition of new environment variables in the README?

Licensing

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@mye956 mye956 marked this pull request as ready for review October 23, 2024 19:13
@mye956 mye956 requested a review from a team as a code owner October 23, 2024 19:13
@mye956 mye956 changed the title [WIP DO NOT REVIEW] Adding unit tests for fault request ordering Adding unit tests for fault request ordering Oct 23, 2024
@mye956 mye956 force-pushed the fault-unit-test branch 3 times, most recently from 7e711d0 to 65171bf Compare October 23, 2024 21:56
@@ -2175,3 +2183,297 @@ func TestCheckNetworkPacketLoss(t *testing.T) {
tcs := generateCheckNetworkPacketLossTestCases()
testNetworkFaultInjectionCommon(t, tcs, NetworkFaultPath(types.PacketLossFaultType, types.CheckNetworkFaultPostfix))
}

func TestNetworkFaultRequestOrdering(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit (non-blocking): There seems to be a lot of repeated code across test cases, especially between L2242-2288 and L2289-L2335. This is not a strict blocker for me but I would prefer if we could look to minimize the amount of repeated code across test cases (for code maintainability purposes, particularly if we expect to add any new fault types in the future) and factor those out into for loop.

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.

4 participants