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

chore(mme): add unit test for fillIpv4 #12100

Merged
merged 1 commit into from Mar 16, 2022

Conversation

LKreutzer
Copy link
Contributor

@LKreutzer LKreutzer commented Mar 14, 2022

Signed-off-by: Lars Kreutzer lars.kreutzer@tngtech.com

Summary

  • Unit test for SpgwServiceImpl::fillIpv4 is added (see fix(mme): Fix network address in create bearer packet filter #12038)
    • SpgwServiceImpl::fillIpv4 is a private function. The only public function that can lead to calls in fillIpv4 is SpgwServiceImpl::CreateBearer and several conditions need to be fulfilled in order to lead to fillIpv4 calls.
    • It seems that the SpgwServiceImpl::fillIpv4 function is not called in the existing unit tests - all OAI unit tests succeed even if e.g. raise(SIGSEGV); is added to the SpgwServiceImpl::fillIpv4 function. The same if true for the SpgwServiceImpl::CreateBearer function which is the only C/C++ function that can lead to calls of the fillIpv4 function (via fillUpPacketFilterContents).
    • SpgwServiceImpl::fillIpv4 is made public in order to enable direct testing and to limit extensive overhead from indirect testing via SpgwServiceImpl::CreateBearer.

Test Plan

  • Run tests make test_oai
  • CI

Additional Information

  • This change is backwards-breaking

@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines. label Mar 14, 2022
@github-actions
Copy link
Contributor

Thanks for opening a PR! 💯

A couple initial guidelines

Howto

  • Reviews. The "Reviewers" listed for this PR are the Magma maintainers who will shepherd it.
  • Checks. All required CI checks must pass before merge.
  • Merge. Once approved and passing CI checks, use the ready2merge label to indicate the maintainers can merge your PR.

More info

Please take a moment to read through the Magma project's

If this is your first Magma PR, also consider reading

@github-actions github-actions bot added the component: agw Access gateway-related issue label Mar 14, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Mar 14, 2022

feg-workflow

    2 files  202 suites   32s ⏱️
362 tests 362 ✔️ 0 💤 0
376 runs  376 ✔️ 0 💤 0

Results for commit d3d2a05.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 14, 2022

agw-workflow

  60 files    87 suites   5m 43s ⏱️
970 tests 961 ✔️ 9 💤 0
971 runs  962 ✔️ 9 💤 0

Results for commit d3d2a05.

♻️ This comment has been updated with latest results.

@LKreutzer LKreutzer marked this pull request as ready for review March 15, 2022 09:22
@LKreutzer LKreutzer requested review from a team and pruthvihebbani March 15, 2022 09:22
Copy link
Contributor

@lionelgo lionelgo left a comment

Choose a reason for hiding this comment

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

A public wrapper dedicated to test bool fillIpv4(...) would not change the existing interfaces.
... but would add an interface on the other side.

Copy link
Member

@themarwhal themarwhal left a comment

Choose a reason for hiding this comment

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

thanks :)

Signed-off-by: Lars Kreutzer <lars.kreutzer@tngtech.com>
@LKreutzer LKreutzer added the ready2merge This PR is ready to be merged (is approved and passes all required checks) label Mar 16, 2022
@themarwhal themarwhal merged commit 9ff0844 into magma:master Mar 16, 2022
vikramgreddy pushed a commit to vikramgreddy/magma that referenced this pull request Mar 23, 2022
Signed-off-by: Lars Kreutzer <lars.kreutzer@tngtech.com>
ardzoht pushed a commit that referenced this pull request Mar 30, 2022
Signed-off-by: Lars Kreutzer <lars.kreutzer@tngtech.com>
emakeev pushed a commit to emakeev/magma that referenced this pull request Aug 5, 2022
Signed-off-by: Lars Kreutzer <lars.kreutzer@tngtech.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: agw Access gateway-related issue ready2merge This PR is ready to be merged (is approved and passes all required checks) size/M Denotes a PR that changes 30-99 lines.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants