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(agw): 'nose' is replaced by 'pytest' for S1AP integration tests #13060

Merged
merged 2 commits into from Jul 15, 2022

Conversation

mpfirrmann
Copy link
Contributor

@mpfirrmann mpfirrmann commented Jun 23, 2022

Summary

Follow-up PR to #13052. The two PRs are independent to not break master by merge order. The duplicate code will be adapted before the second merge.
After that, there will be a cleanup PR for the remaining occurrences of nose/nosetests and the handling of coverage.
Additionally ...
... pytest did complain about some python imports --> adapted according to the other tests.
... flaky in pytest is enabled by default (https://github.com/box/flaky#activating-the-plugin) --> argument is removed

Test Plan

With all three VMs for AGW testing:
vagrant@magma-test:~/magma/lte/gateway/python/integ_test$ make integ_test

LTE integ_test CI runs on my fork:
https://github.com/mpfirrmann/magma/actions/runs/2548683727
https://github.com/mpfirrmann/magma/actions/runs/2550991806
A different unit test fails at both runs, but probably not due to the code change here.

Additional Information

The test results are stored in .xml files. On this basis, CI html reports are created. The style of the result .xml changes slightly with the change from XUnit XML to JUnit XML.
For s1aptests/test_attach_detatch.xml:
current XUnit XML

<testsuite name="nosetests" tests="1" errors="0" failures="0" skip="0">
<testcase classname="test_attach_detach.TestAttachDetach" name="test_attach_detach" time="5.273">
<system-out>
<![CDATA[ Test Case Execution Count: 1 Start time 06:56:28 ************************* Enb tester config AGW is stateless APN Correction configured Health service is disabled Using subscriber IMSI IMSI001010000000001 Using IMEI 3805468432113171 Using subscriber IMSI IMSI001010000000002 Using IMEI 3805468432113172 ************************* UE device config for ue_id 1 ************************* UE device config for ue_id 2 ************************* Configuring IP block ************************* Waiting for IP changes to propagate ************************* S1 setup ************************* UE App config ************************* Running End to End attach for UE id 1 ************************* Running UE detach for UE id 1 ************************* Running End to End attach for UE id 2 ************************* Running UE detach for UE id 2 ************************* send SCTP SHUTDOWN Keys left in Redis (list should be empty)[ ] Entries in s1ap_imsi_map (should be zero): 0 Entries left in hashtables (should be zero): 0 Entries in mme_ueip_imsi_map (should be zero): 0 ]]>
</system-out>
</testcase>
</testsuite>

new JUnit XML

<testsuites>
<testsuite name="pytest" errors="0" failures="0" skipped="0" tests="1" time="5.188" timestamp="2022-07-06T07:36:02.476018" hostname="magma-test">
<testcase classname="s1aptests.test_attach_detach.TestAttachDetach" name="test_attach_detach" time="4.998">
<system-out>--------------------------------- Captured Out --------------------------------- Test Case Execution Count: 1 Start time 07:36:02 ************************* Enb tester config AGW is stateless APN Correction configured Health service is disabled Using subscriber IMSI IMSI001010000000001 Using IMEI 3805468432113171 Using subscriber IMSI IMSI001010000000002 Using IMEI 3805468432113172 ************************* UE device config for ue_id 1 ************************* UE device config for ue_id 2 ************************* Configuring IP block ************************* Waiting for IP changes to propagate ************************* S1 setup ************************* UE App config ************************* Running End to End attach for UE id 1 ************************* Running UE detach for UE id 1 ************************* Running End to End attach for UE id 2 ************************* Running UE detach for UE id 2 ************************* send SCTP SHUTDOWN Keys left in Redis (list should be empty)[ ] Entries in s1ap_imsi_map (should be zero): 0 Entries left in hashtables (should be zero): 0 Entries in mme_ueip_imsi_map (should be zero): 0 </system-out>
</testcase>
</testsuite>
</testsuites>
  • This change is backwards-breaking

@mpfirrmann mpfirrmann requested a review from a team June 23, 2022 09:45
@mpfirrmann mpfirrmann requested a review from a team as a code owner June 23, 2022 09:45
@pull-request-size pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines. label Jun 23, 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 Jun 23, 2022
@pull-request-size pull-request-size bot added size/M Denotes a PR that changes 30-99 lines. and removed size/S Denotes a PR that changes 10-29 lines. labels Jun 23, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jun 23, 2022

feg-workflow

    2 files  203 suites   40s ⏱️
374 tests 374 ✔️ 0 💤 0
388 runs  388 ✔️ 0 💤 0

Results for commit 725557d.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 23, 2022

dp-workflow

14 tests   14 ✔️  3m 40s ⏱️
  1 suites    0 💤
  1 files      0

Results for commit 725557d.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 23, 2022

agw-workflow

615 tests   611 ✔️  3m 52s ⏱️
    2 suites      4 💤
    2 files        0

Results for commit 725557d.

♻️ This comment has been updated with latest results.

@sebathomas
Copy link
Contributor

It might be good to have a CI run of the integration tests on your fork.

@mpfirrmann mpfirrmann removed the request for review from koolzz June 27, 2022 09:07
@VinashakAnkitAman
Copy link
Member

VinashakAnkitAman commented Jul 5, 2022

LGTM

On CI side, are the xml reports same from existing nosetest-xunit and pytest-junit?

We currently use xunit-viewer to generate CI html reports (https://storage.googleapis.com/magma-ci.appspot.com/lte_integ_test_a4934e6e076a23264ccaa65df327bf119857fe26.html). Is that compatible with new xml files generated by pytest-junit?

@mpfirrmann
Copy link
Contributor Author

@VinashakAnkitAman, that is good to know. What does the CI currently check for in the .xml files? From what I saw, the general result plus the stdout is written there. This can be done with pytest as well. Added example outputs to the PR description.

@Neudrino
Copy link
Contributor

Neudrino commented Jul 6, 2022

I think the important thing about the output format is, that it integrates well with Github actions result display as the community platform and infrastructure. E.g. as for this job. The integration there seems to be quite good already.

@mpfirrmann
Copy link
Contributor Author

@VinashakAnkitAman, did you by chance had time to look at the .xml outputs of the pytest tests and how they would fit into the CI? With your approval we could merge this PR.

Signed-off-by: Marco Pfirrmann <marco.pfirrmann@tngtech.com>
Signed-off-by: Marco Pfirrmann <marco.pfirrmann@tngtech.com>
@VinashakAnkitAman
Copy link
Member

@VinashakAnkitAman, did you by chance had time to look at the .xml outputs of the pytest tests and how they would fit into the CI? With your approval we could merge this PR.

Hi @mpfirrmann Right now my local xunit-viewer package is broken and I could not verify the html generation with the xml added here. However the junit-viewer package itself has been deprecated and xunit-viewer will be used for the junit generated xml files as well as per the documentation https://www.npmjs.com/package/junit-viewer and https://github.com/lukejpreston/xunit-viewer. So, from my end, I think we can go ahead and merge the PR.

However, for double check, It would be helpful if @ardzoht or @tmdzk can suggest any method with which we can generate CI reports over lte integ test for this PR to see if xunit-viewer is generating the html reports properly for junit generated xml files.

@ardzoht
Copy link
Contributor

ardzoht commented Jul 15, 2022

@VinashakAnkitAman as discussed offline, let's merge the PR and monitor closely the HTML results, for validating the updates. We can revert in case there's any issue introduced by the update.

Copy link
Member

@VinashakAnkitAman VinashakAnkitAman left a comment

Choose a reason for hiding this comment

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

LGTM. Will be verified in CI for HTML report generation after merge.

@VinashakAnkitAman VinashakAnkitAman merged commit a484080 into magma:master Jul 15, 2022
emakeev pushed a commit to emakeev/magma that referenced this pull request Aug 5, 2022
…magma#13060)

* chore(agw): import commands are adapted

Signed-off-by: Marco Pfirrmann <marco.pfirrmann@tngtech.com>

* chore(agw): 'nose' is replaced by 'pytest' for S1AP integration tests

Signed-off-by: Marco Pfirrmann <marco.pfirrmann@tngtech.com>
@mpfirrmann mpfirrmann deleted the nose_integ_test branch September 21, 2022 15:40
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 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

5 participants