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

Read suites from Kubernetes testrun resource #74

Merged
merged 8 commits into from
Dec 2, 2024

Conversation

andmat900
Copy link
Contributor

@andmat900 andmat900 commented Nov 19, 2024

Applicable Issues

Description of the Change

This change enables ETOS Suite Runner to read suites from Kubernetes' TestRun.

Alternate Designs

Possible Drawbacks

Sign-off

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or

(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or

(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.

(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.

Signed-off-by: Andrei Matveyeu, [email protected]

@andmat900 andmat900 requested a review from a team as a code owner November 19, 2024 11:33
@andmat900 andmat900 requested review from t-persson and fredjn and removed request for a team November 19, 2024 11:33
Copy link
Collaborator

@t-persson t-persson left a comment

Choose a reason for hiding this comment

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

Rebase

@andmat900 andmat900 force-pushed the 20241119_read_testruns branch from d44fcb9 to cead2dc Compare November 28, 2024 14:47
Copy link
Collaborator

@t-persson t-persson left a comment

Choose a reason for hiding this comment

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

Has this change been tested? It should not work.

Comment on lines 155 to 165
tercc = EiffelTestExecutionRecipeCollectionCreatedEvent()
tercc_json = None
if os.getenv("TERCC") is not None:
# first option for backwards compatibility
self.logger.info("Reading TERCC from environment variable")
tercc_json = json.loads(os.getenv("TERCC", "{}"))
tercc.rebuild(tercc_json)
else:
# requires testrun custom resource defined in Kubernetes
self.logger.info("Reading TERCC from Kubernetes testrun resource")
tercc = TestRun(Kubernetes()).get(os.getenv("TESTRUN"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

A few problems with this.

  1. This property should only be used as a fallback for when we're not running in the controller environment (line 123).
  2. The TestRun resource from Kubernetes is not equivalent to a TERCC.
  3. The code for requesting a TestRun resource from Kubernetes instead of loading the TERCC environment variable as a TestRun should be in the test_suite property (line 174)

@andmat900 andmat900 force-pushed the 20241119_read_testruns branch from cf63faf to e7b07fa Compare November 29, 2024 10:44
Comment on lines 162 to 163
if isinstance(tercc, list):
test_suite = [Suite(**suite) for suite in tercc]
Copy link
Collaborator

Choose a reason for hiding this comment

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

This part was the part that handled TESTRUN before. It shouldn't be needed here.

Comment on lines 182 to 185
if os.environ.get("TERCC") is not None:
self.__test_suite = self._get_test_suite_list_from_tercc()
else:
test_suite = self._eiffel_test_suite(tercc)
# The dataset is not necessary for the suite runner.
self.__test_suite = [Suite.from_tercc(suite, {}) for suite in test_suite]
self.__test_suite = self._get_test_suite_list_from_kubernetes_testrun()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not just

if testrun_id := os.getenv("TESTRUN") is not None:
    testrun = TestRun(Kubernetes()).get(testrun_id)
    self.__test_suite = testrun.spec.suites
else:
    self.__test_suite = self._eiffel_test_suite(os.getenv("TERCC", "{}"))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Parentheses are needed in the if-line, because otherwise testrun_id evaluates to a boolean.

The else block seems to need this line too:

test_suite = [Suite.from_tercc(suite, {}) for suite in test_suite]

@andmat900 andmat900 changed the title Read TERCC from Kubernetes instead of environment Read suites from Kubernetes testrun resource Nov 29, 2024
@andmat900 andmat900 merged commit 0b1508e into eiffel-community:main Dec 2, 2024
3 checks passed
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.

2 participants