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

[DPE-2095] integration tests to pytest #16

Merged
merged 5 commits into from
Jun 16, 2023

Conversation

juditnovak
Copy link
Contributor

@juditnovak juditnovak commented Jun 13, 2023

Blindly moving from unittest to pytest, as a pre-stage to complete DPE-2021.
Related, dependent PR is #17

Stateful, "cleaning up after myself" tests not used anymore.
Instead we have reliable fixtures always ensuring a clean state (in terms of namespaces).
(Note: So far I see no desperate need to clean up the default namespace, but can be added if needed)

Furthermore tests/__init__.py is cleaned up (none of the content was needed there).

While it may be difficult to review changes, note that test coverage is equal to the last one before this PR:

---------- coverage: platform linux, python 3.9.16-final-0 -----------
Name                                      Stmts   Miss  Cover
-------------------------------------------------------------
spark8t/__init__.py                           0      0   100%
spark8t/cli/__init__.py                       3      3     0%
spark8t/cli/params.py                        29     29     0%
spark8t/cli/pyspark.py                       15     15     0%
spark8t/cli/service_account_registry.py      76     76     0%
spark8t/cli/spark_shell.py                   15     15     0%
spark8t/cli/spark_submit.py                  15     15     0%
spark8t/domain.py                           161     27    83%
spark8t/exceptions.py                        10      1    90%
spark8t/literals.py                           3      0   100%
spark8t/services.py                         527    202    62%
spark8t/utils.py                             93     27    71%
-------------------------------------------------------------
TOTAL                                       947    410    57%

================== 12 passed, 2 warnings in 138.45s (0:02:18) ==================
  integration: OK (144.86=setup[1.35]+cmd[3.22,0.59,139.71] seconds)
  congratulations :) (144.99 seconds)

New execution

---------- coverage: platform linux, python 3.9.17-final-0 -----------
Name                                      Stmts   Miss  Cover
-------------------------------------------------------------
spark8t/__init__.py                           0      0   100%
spark8t/cli/__init__.py                       3      3     0%
spark8t/cli/params.py                        29     29     0%
spark8t/cli/pyspark.py                       15     15     0%
spark8t/cli/service_account_registry.py      76     76     0%
spark8t/cli/spark_shell.py                   15     15     0%
spark8t/cli/spark_submit.py                  15     15     0%
spark8t/domain.py                           161     27    83%
spark8t/exceptions.py                        10      1    90%
spark8t/literals.py                           3      0   100%
spark8t/services.py                         527    202    62%
spark8t/utils.py                             93     27    71%
-------------------------------------------------------------
TOTAL                                       947    410    57%

================== 12 passed, 2 warnings in 133.44s (0:02:13) ==================
  integration: OK (142.51=setup[1.63]+cmd[3.67,1.18,136.03] seconds)
  congratulations :) (142.66 seconds)

@juditnovak juditnovak force-pushed the DPE-2095_integration_tests_to_pytest branch 2 times, most recently from 0ecc707 to 7704604 Compare June 13, 2023 22:39
dict(os.environ) | {"KUBECONFIG": f"{os.environ['HOME']}/.kube/config"}
@pytest.mark.parametrize(
"namespace, user",
[("default-namespace", "spark"), ("spark-namespace", "spark-user")],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

[note] I'm much wondering that we should not be using the same namespace across multiple tests (except default), as it prevents us from being able to run our tests simultaneously.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, makes sense, we could randomize this, but let's do this later. Not needed at this point

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's kinda half-true -- well true as long as there are no LightKube tests ;-)

Turns out that LightKube is a lot slower in removing namespaces than doing it directly with kubectl. Thus if you want to reuse the name of a namespace (that's just been cleaned up) in a subsequential test, it will fail with an error indicating that the namespace is just being deleted.

You'll see it later, when LightKube tests are finally activated and passing, I had to randomize the names manually already. (The change was introduced "passively" (xfail) in PR #17 -- however I'm using the below references from PR https://github.com/canonical/spark-k8s-toolkit-py/pull/19/files#diff-cdd7ffd3e23e3cfe0690b94308e2a7b70a22c89a08aa78b06396b7e382c38c47L71 where the tests are actually enabled and passing.)

I'll do this on a nice centralized fixture level later.
(This PR is to be kept as clean as possible as a direct mapping between unittest -> pytest).

@juditnovak juditnovak requested a review from deusebio June 13, 2023 23:27
@juditnovak juditnovak marked this pull request as ready for review June 13, 2023 23:32
@juditnovak juditnovak force-pushed the DPE-2095_integration_tests_to_pytest branch 2 times, most recently from fd81bf1 to cbf7d15 Compare June 16, 2023 00:26
Copy link
Contributor

@deusebio deusebio left a comment

Choose a reason for hiding this comment

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

LGTM

@juditnovak juditnovak force-pushed the DPE-2095_integration_tests_to_pytest branch from cbf7d15 to 8ee05e2 Compare June 16, 2023 15:02
@juditnovak juditnovak merged commit 709e94f into main Jun 16, 2023
4 checks passed
@juditnovak juditnovak deleted the DPE-2095_integration_tests_to_pytest branch June 16, 2023 15:15
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.

None yet

3 participants