Skip to content

e2e: make tests divided into smaller parts (DLB, DSA, IAA, QAT) #1263

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

Merged
merged 4 commits into from
Feb 16, 2023
Merged

e2e: make tests divided into smaller parts (DLB, DSA, IAA, QAT) #1263

merged 4 commits into from
Feb 16, 2023

Conversation

hj-johannes-lee
Copy link
Contributor

@hj-johannes-lee hj-johannes-lee commented Dec 6, 2022

Restructure e2e tests from a single It() as follows:

Describe("device")
 BeforeEach("deploys plugin")
  Context("When device resources are available")
   BeforeEach("checks if resources are available")
    It("runs a pod requesting resources")
    It("runs another pod requesting resources if there is")

Signed-off-by: Hyeongju Johannes Lee hyeongju.lee@intel.com

@hj-johannes-lee hj-johannes-lee marked this pull request as draft December 6, 2022 12:52
@codecov-commenter
Copy link

codecov-commenter commented Dec 12, 2022

Codecov Report

Merging #1263 (e2c7fad) into main (f3b4294) will not change coverage.
The diff coverage is n/a.

❗ Current head e2c7fad differs from pull request most recent head 8cb54f2. Consider uploading reports for the commit 8cb54f2 to get more accurate results

@@           Coverage Diff           @@
##             main    #1263   +/-   ##
=======================================
  Coverage   51.17%   51.17%           
=======================================
  Files          44       44           
  Lines        4879     4879           
=======================================
  Hits         2497     2497           
  Misses       2239     2239           
  Partials      143      143           
Impacted Files Coverage Δ
pkg/controllers/reconciler.go 4.25% <ø> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@mythi
Copy link
Contributor

mythi commented Dec 13, 2022

Additionally, fix grammar mistakes and typos

these can go to a separate PR

@hj-johannes-lee
Copy link
Contributor Author

@mythi Sure, i will divide them.!

@hj-johannes-lee hj-johannes-lee changed the title e2e: make tests divided into smaller parts e2e: make tests divided into smaller parts (DLB, DSA, IAA, QAT) Jan 19, 2023
@hj-johannes-lee hj-johannes-lee marked this pull request as ready for review January 20, 2023 21:35
@hj-johannes-lee hj-johannes-lee requested a review from kad as a code owner January 20, 2023 21:35
@hj-johannes-lee
Copy link
Contributor Author

Updates:
FPGA has some problem, so I thought it would be good to do first the 4 device plugins that are used in SPR (DLB, DSA, IAA, QAT).

To do (maybe in another PR):
QAT4 - cy device is recognized as generic inside a VM.
DLB - pf device is recognized as vf inside a VM.

We need to deal with this matter.

@hj-johannes-lee
Copy link
Contributor Author

Could you review? :)

@hj-johannes-lee
Copy link
Contributor Author

@mythi @ozhuraki Could you review? :)

@mythi
Copy link
Contributor

mythi commented Feb 2, 2023

Is there a possibility this breaks if Ginkgo randomizes things differently? Now the test cases have a dependency and it might be that the demo app gets deployed before the plugin?

@hj-johannes-lee
Copy link
Contributor Author

@mythi yes, I thought about that matter,, and there is 'ginkgo. Order' that makes everything run in order. Following ones are skipped in case previous It() fails.
I thought I put them already since no test got failed. Let me put it.

@mythi
Copy link
Contributor

mythi commented Feb 2, 2023

there is 'ginkgo. Order'

randomisation is on purpose to ensure the tests are correctly written so I don't think we want to disable it

@hj-johannes-lee
Copy link
Contributor Author

@mythi Umm, did I understand what you meant correctly? What 'randomization' do we have for now? We have only one It()!

@mythi
Copy link
Contributor

mythi commented Feb 3, 2023

@mythi Umm, did I understand what you meant correctly? What 'randomization' do we have for now? We have only one It()!

I was reading this before asking my question https://onsi.github.io/ginkgo/#spec-randomization

@hj-johannes-lee
Copy link
Contributor Author

I then suggest this way:

Describe ("device")

  • BeforeEach("deploys plugin")
    • Context("When device resources are available")
      • BeforeEach("checks if resources are available")
      • It("runs a pod requesting resources")
      • It("runs another pod requesting resources if there is")

In this way, we can randomize specs and does not cause any problem no matter how many It()s exists.

@mythi
Copy link
Contributor

mythi commented Feb 4, 2023

In this way, we can randomize specs and does not cause any problem no matter how many It()s exists.

The proposal looks much better and closer to what I originally had in mind with #1143

@hj-johannes-lee
Copy link
Contributor Author

@mythi Thanks! :) I guess all ci passed.! Please review the code! :)
For the matter of GPU and FPGA, can I do in another PR later?

@mythi
Copy link
Contributor

mythi commented Feb 6, 2023

@mythi Thanks! :) I guess all ci passed.! Please review the code! :) For the matter of GPU and FPGA, can I do in another PR later?

It's fine. For this PR, I'd suggest we change things to one commit per device.

@hj-johannes-lee
Copy link
Contributor Author

@mythi Done.! What else would this pr you think need? :)

@hj-johannes-lee
Copy link
Contributor Author

@ozhuraki Could you review?

mythi
mythi previously approved these changes Feb 9, 2023
@hj-johannes-lee
Copy link
Contributor Author

After this get merged, let me also make a pr for the improvement of documentations for e2e tests.!

Structure is as follows:
Describe("DLB plugin")
  BeforeEach("deploys plugin")
    Context("When device resources are available")
      BeforeEach("checks if resources are available")
        It("runs a pod requesting resources")

Signed-off-by: Hyeongju Johannes Lee <hyeongju.lee@intel.com>
Structure is as follows:
Describe("DSA plugin")
  Describe("without using operator")
    BeforeEach("deploys plugin")
      Context("When device resources are available")
        BeforeEach("checks if resources are available")
          It("runs a pod requesting resources")
  Describe("with using operator")
    It("deploys with operator")

Signed-off-by: Hyeongju Johannes Lee <hyeongju.lee@intel.com>
Structure is as follows:
Describe("IAA plugin")
  Describe("without using operator")
    BeforeEach("deploys plugin")
      Context("When device resources are available")
        BeforeEach("checks if resources are available")
          It("runs a pod requesting resources")
  Describe("with using operator")
    It("deploys with operator")

Signed-off-by: Hyeongju Johannes Lee <hyeongju.lee@intel.com>
Structure is as follows:
Describe("QAT plugin")
  BeforeEach("deploys plugin")
    Context("When device resources are available")
      BeforeEach("checks if resources are available")
        It("runs a pod requesting resources")
        It("runs another pod requesting resources if there is")

Signed-off-by: Hyeongju Johannes Lee <hyeongju.lee@intel.com>
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