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

Modularize implementation of tests #1861 #1867

Merged
merged 1 commit into from
Feb 28, 2024

Conversation

martin-mat
Copy link
Collaborator

Description

Modularized test im plementation:

  • centralized test result output

  • remove repeated code/strings/emojis from test implementation

  • category emojis are taken based on test category in points.yml

  • the adaptations require adaptation (simplification and removal of code) of every test case.

  • This is a WIP pull request for purpose of review of the chanegs. Only test cases in workload/observability.cr are adapted. What is missing is to adapt all remaining tests (after this one goes through the review process to avoid changing many files during the review). Do not merge for now.

  • note that it was also required to fix some of [BUG] wrong usage of named arguments across the code #1864 as the code stopped working after adding arguments to some methods.

  • also note that ResultStatus enum was moved 2 levels up in hierarchy to be better usable.

Issues:

Refs: #1861

How has this been tested:

  • Covered by existing integration testing
  • Added integration testing to cover
  • Verified all A/C passes
    • develop
    • master
    • tag/other branch
  • Test environment
    • Shared Packet K8s cluster
    • New Packet K8s cluster
    • Kind cluster
  • Have not tested

Types of changes:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

Checklist:

Documentation

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • No updates required.

Code Review

  • Does the test handle fatal exceptions, ie. rescue block

Issue

  • Tasks in issue are checked off

@taylor taylor requested a review from HashNuke January 26, 2024 16:43
@agentpoyo agentpoyo changed the title Modularize implemetation of tests #1861 WIP: Modularize implementation of tests #1861 Jan 26, 2024
Copy link
Collaborator

@HashNuke HashNuke left a comment

Choose a reason for hiding this comment

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

LGTM 👍

A suggestion we could take up later: Move the test-specific emojis for tests to the points.yml file since test name and category is also maintained there.

@martin-mat martin-mat force-pushed the tc_modularized branch 2 times, most recently from a9b8559 to 7febb43 Compare February 20, 2024 10:55
@martin-mat martin-mat changed the title WIP: Modularize implementation of tests #1861 Modularize implementation of tests #1861 Feb 20, 2024
@HashNuke
Copy link
Collaborator

@martin-mat Could you please update this PR with the latest changes from the main branch and resolve conflicts? Merging latest main would also fix any build issues that aren't related to this PR.

Thanks.

Copy link
Collaborator

@agentpoyo agentpoyo left a comment

Choose a reason for hiding this comment

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

LGTM, https://github.com/cnti-testcatalog/testsuite/actions/runs/8067488377 is passing github actions as well. If you create a pull request, we can verify and this should close out #1867

@HashNuke HashNuke merged commit 04c37af into cnti-testcatalog:main Feb 28, 2024
110 of 190 checks passed
@HashNuke
Copy link
Collaborator

Merging since build is passing when pushed from a branch in the repo - https://github.com/cnti-testcatalog/testsuite/actions/runs/8067488377

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