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

Bug Fix: Keploy test runs succesfully with no test cases #1597

Merged
merged 16 commits into from
Apr 15, 2024

Conversation

Akash-Singh04
Copy link
Contributor

@Akash-Singh04 Akash-Singh04 commented Feb 18, 2024

Related Issue

-Bug Fix

Closes: #1270 #1750

Describe the changes you've made

  1. Added check to ensure keploy test informs the user that no test cases were found instead of completing the test successfully.
  2. If not all test cases are empty, keploy test runs normally and informs user of the empty test cases.
  3. Added check to ensure keploy test informs user of missing Keploy Folder and exits gracefully.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, local variables)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Please let us know if any test cases are added

NIL

Describe if there is any unusual behaviour of your code(Write NA if there isn't)

NIL

Checklist:

  • My code follows the style guidelines of this project.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.

Screenshots (if any)

  1. image
  2. image
  3. image

Signed-off-by: Akash <akashsingh2210670@gmail.com>
@Akash-Singh04
Copy link
Contributor Author

@charankamarapu Let me know if this implementation is apt!

@charankamarapu
Copy link
Member

Please update the branch . And also have the many number of test-set then how are you going to approach this . Lets say I have 10 test-sets in which only one is empty.

Copy link
Member

@charankamarapu charankamarapu left a comment

Choose a reason for hiding this comment

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

there are no file changes

@Akash-Singh04
Copy link
Contributor Author

@charankamarapu After the v2 refactoring, i have added warnings so that the user is warned about missing keploy folder, and if only a single test set is empty, the execution of other test sets continues normally whilst informing the user of the missing test set. Please review the PR and refer to the updated pr description for the screenshots

Signed-off-by: Akash Singh <akashsingh2210670@gmail.com>
Copy link
Member

@charankamarapu charankamarapu left a comment

Choose a reason for hiding this comment

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

Please address the comments

pkg/service/replay/replay.go Outdated Show resolved Hide resolved
pkg/service/replay/replay.go Show resolved Hide resolved
@Akash-Singh04
Copy link
Contributor Author

@charankamarapu Your requested changes have been made

Copy link
Member

@charankamarapu charankamarapu left a comment

Choose a reason for hiding this comment

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

Please address the comments

if _, err := os.Stat(c.cfg.Path); os.IsNotExist(err) {
recordCmd := models.HighlightGrayString("keploy record")
errMsg := fmt.Sprintf("Keploy folder not found. Please record testcases using %s command", recordCmd)
utils.LogError(c.logger, err, errMsg)
Copy link
Member

Choose a reason for hiding this comment

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

Hey just saw the screenshot and I think the error shouldn't include stat error and it will be good if error is "no test-sets found" rather than "no keploy folder" because keploy folder has no usecase to user but test-sets does. Please update the screenshots after this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Refactored and updated screenshot no.3

Signed-off-by: Akash Singh <akashsingh2210670@gmail.com>
@Akash-Singh04
Copy link
Contributor Author

@charankamarapu Your requested changes have been made

Copy link
Member

@charankamarapu charankamarapu left a comment

Choose a reason for hiding this comment

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

LGTM

@charankamarapu charankamarapu merged commit 714ad96 into keploy:main Apr 15, 2024
14 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Apr 15, 2024
@Akash-Singh04 Akash-Singh04 deleted the testcmdbug branch April 15, 2024 08:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

keploy test completes successfully in projects without test sets
4 participants