Skip to content

[pytest]: Re-use DVS container when possible#1816

Merged
theasianpianist merged 24 commits intosonic-net:masterfrom
theasianpianist:persistent-dvs
Sep 28, 2021
Merged

[pytest]: Re-use DVS container when possible#1816
theasianpianist merged 24 commits intosonic-net:masterfrom
theasianpianist:persistent-dvs

Conversation

@theasianpianist
Copy link
Contributor

@theasianpianist theasianpianist commented Jul 9, 2021

Signed-off-by: Lawrence Lee [email protected]

What I did

  • If the fake_platform does not change between test modules, re-use the same DVS container (but still restart it between modules and do some cleanup/re-init to ensure a consistent start state for each test module)
  • Add a CLI option --force-recreate-dvs to revert to the previous behavior of creating a new DVS per test module

Why I did it

  • Reduce the total number of containers used to speed up gcov

How I verified it

  • Run the pytests locally. Upon reaching the second module, verify with docker ps that the DVS container being used is not newly created.

Details if related

- If the fake_platform does not change between test modules, re-use the same DVS container

Signed-off-by: Lawrence Lee <[email protected]>
@theasianpianist theasianpianist requested a review from prsunny July 9, 2021 19:12
@lgtm-com
Copy link

lgtm-com bot commented Jul 9, 2021

This pull request introduces 1 alert when merging 9706eaa into 64e33b3 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@prsunny prsunny requested a review from lguohan July 13, 2021 00:39
@theasianpianist
Copy link
Contributor Author

/Azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@theasianpianist
Copy link
Contributor Author

/Azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@prsunny prsunny mentioned this pull request Jul 16, 2021
@pettershao-ragilenetworks
Copy link
Contributor

@prsunny From test result, if vs docker restart, the gcda info will loss, can change to keep docker active?

@lgtm-com
Copy link

lgtm-com bot commented Jul 20, 2021

This pull request introduces 1 alert when merging e3b6e70 into c805021 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@pettershao-ragilenetworks
Copy link
Contributor

@theasianpianist
Copy link
Contributor Author

/Azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@lgtm-com
Copy link

lgtm-com bot commented Jul 28, 2021

This pull request introduces 1 alert when merging 6d2ecd5 into ec104c1 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@pettershao-ragilenetworks
Copy link
Contributor

@theasianpianist
Copy link
Contributor Author

/Azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@lgtm-com
Copy link

lgtm-com bot commented Aug 6, 2021

This pull request introduces 1 alert when merging 8074c39 into df96059 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@pettershao-ragilenetworks
Copy link
Contributor

pettershao-ragilenetworks commented Aug 8, 2021

@theasianpianist help fix lgtm, thanks! and test time reach to 3h, please check https://dev.azure.com/mssonic/build/_build/results?buildId=27958&view=logs&s=859b8d9a-8fd6-5a5c-6f5e-f84f1990894e, other thing are all fine!

@lgtm-com
Copy link

lgtm-com bot commented Aug 9, 2021

This pull request introduces 1 alert when merging 88078ed into df96059 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@prsunny
Copy link
Collaborator

prsunny commented Aug 10, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@prsunny
Copy link
Collaborator

prsunny commented Aug 11, 2021

@theasianpianist , can you please take a look at the failure?

@pettershao-ragilenetworks
Copy link
Contributor

@theasianpianist https://dev.azure.com/mssonic/build/_build/results?buildId=34436&view=results result seems fine, please help fix LGTM, thanks!

@lgtm-com
Copy link

lgtm-com bot commented Sep 8, 2021

This pull request introduces 1 alert when merging f8706df into 254bc12 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@lgtm-com
Copy link

lgtm-com bot commented Sep 15, 2021

This pull request introduces 1 alert when merging 386b15d into 8cf355d - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@lgtm-com
Copy link

lgtm-com bot commented Sep 15, 2021

This pull request introduces 2 alerts when merging 033d699 into d01524d - view on LGTM.com

new alerts:

  • 1 for Unused local variable
  • 1 for Unused import

Signed-off-by: Lawrence Lee <[email protected]>
@theasianpianist
Copy link
Contributor Author

@pettershao-ragilenetworks finally able to achieve 100% test pass rate, can you check if the latest changes work with gcov? Unfortunately the LGTM alert for the unused local variable is a false positive and for some reason the suppression comment I added is not taking effect.

@lgtm-com
Copy link

lgtm-com bot commented Sep 16, 2021

This pull request introduces 1 alert when merging 97489fc into d01524d - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@theasianpianist
Copy link
Contributor Author

/Azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@theasianpianist
Copy link
Contributor Author

/Azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@theasianpianist
Copy link
Contributor Author

/Azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@theasianpianist
Copy link
Contributor Author

/Azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@theasianpianist
Copy link
Contributor Author

/Azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@theasianpianist theasianpianist merged commit a031542 into sonic-net:master Sep 28, 2021
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-swss that referenced this pull request Oct 5, 2021
- If the fake_platform does not change between test modules, re-use the same DVS container (but still restart it between modules and do some cleanup/re-init to ensure a consistent start state for each test module)
- Add a CLI option --force-recreate-dvs to revert to the previous behavior of creating a new DVS per test module

Signed-off-by: Lawrence Lee <[email protected]>
vaibhavhd added a commit to vaibhavhd/sonic-swss that referenced this pull request Oct 7, 2021
vaibhavhd added a commit to sonic-net/sonic-buildimage that referenced this pull request Oct 9, 2021
[Submodule advance sonic-swss]
Include below commits to master image:

Cache routes for single nexthop for faster retrieval sonic-net/sonic-swss#1922
Reduce route count for route perf test (sonic-net/sonic-swss#1928)
[pytest]: Re-use DVS container when possible (sonic-net/sonic-swss#1816)
[PORTSYNCD] when no ports on config db on init, continue and set Port… (sonic-net/sonic-swss#1861)
[gearbox] Add gearbox unit test (sonic-net/sonic-swss#1920)
Reverted skipped test_buffer_dynamic test cases (sonic-net/sonic-swss#1937)
Revert "[buffer orch] Bugfix: Don't query counter SAI_BUFFER_POOL_STA… (sonic-net/sonic-swss#1945)
Janetxxx pushed a commit to Janetxxx/sonic-swss that referenced this pull request Nov 10, 2025
- If the fake_platform does not change between test modules, re-use the same DVS container (but still restart it between modules and do some cleanup/re-init to ensure a consistent start state for each test module)
- Add a CLI option --force-recreate-dvs to revert to the previous behavior of creating a new DVS per test module

Signed-off-by: Lawrence Lee <[email protected]>
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.

3 participants