Feature: added generateTestReport flag #1513

Yaxhveer · 2024-02-06T19:17:43Z

Related Issue

Info about Issue or bug

Closes: #1393

Describe the changes you've made

Added a Flag --generateTestReports to keploy test and config file which is true by default.

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

Please describe the tests(if any). Provide instructions how its affecting the coverage.

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

A clear and concise description of it.

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)

Original	Updated
original screenshot	updated screenshot

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

Yaxhveer · 2024-02-06T19:30:06Z

@gouravkrosx test reports will be generated in serve command since they are used to show status.

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

gouravkrosx · 2024-02-07T08:01:05Z

@Yaxhveer
You can delete the TestReport folder and files after the completion in case of the serve command.

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

Yaxhveer · 2024-02-07T08:28:40Z

@gouravkrosx Please review the changes.

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

PranshuSrivastava · 2024-02-07T16:07:11Z

cmd/test.go

@@ -384,17 +391,21 @@ func (t *Test) GetCmd() *cobra.Command {
 			}

 			path += "/keploy"
+			t.logger.Info("", zap.Any("keploy test and mock path", path))


Why has this line been added

Actually t.logger.Info("", zap.Any("keploy test and mock path", path), zap.Any("keploy testReport path", testReportPath)) was a combined log and I have divided it into two different log for path and testReport Path.

PranshuSrivastava · 2024-02-07T16:08:27Z

cmd/test.go

 			}
-
-			t.logger.Info("", zap.Any("keploy test and mock path", path), zap.Any("keploy testReport path", testReportPath))


Why have these essential logs been removed?

Similar reason divided it into two different logs

PranshuSrivastava · 2024-02-07T16:14:20Z

pkg/service/test/test.go

-	err = cfg.TestReportFS.Write(context.Background(), cfg.TestReportPath, cfg.TestReport)
-
-	t.logger.Info("test report for "+cfg.TestSet+": ", zap.Any("name: ", cfg.TestReport.Name), zap.Any("path: ", cfg.Path+"/"+cfg.TestReport.Name))
+	if cfg.GenerateTestReport {


if we are not generating the testreport we should atleast log once that we are skipping the test report since the flag has been set to false. Do this in all the services with this if condition

PranshuSrivastava · 2024-02-07T16:15:43Z

pkg/graph/serve.go

 	var ps *proxy.ProxySet

+	defer pkg.DeleteTestReports(g.logger, generateTestReport)


why exactly are we deleting the reports at the end of the function?

serve command require testReports to show status. So I removed the testReports when function ends

Can't we just get the status without writing to a file, or checking the condition before writing to the file?

Actually this approach was recommended here #1513 (comment).

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

PranshuSrivastava · 2024-02-09T07:16:27Z

@Yaxhveer the functionality is working correctly, there is just one problem with the logs

Why is this same log being printed 3 times? It should be printed only once, or if you are logging three logs, they should atleast contain useful information that was not in the previous log, although I don't think that will be the case.

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

Yaxhveer · 2024-02-09T07:40:47Z

@PranshuSrivastava fixed the extra logs issue

PranshuSrivastava

LGTM

Yaxhveer added 2 commits February 7, 2024 00:43

added generateTestReport flag

c112f06

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

error fixed

bede2c3

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

error fixed

26befdd

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

Yaxhveer added 2 commits February 7, 2024 13:49

delete testReports in serve command

4a2ce9d

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

error fixed

b41fb13

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

Yaxhveer added 2 commits February 7, 2024 16:55

fixed issues

9a04746

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

Merge branch 'main' into test-report-optional

6c669db

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

PranshuSrivastava requested changes Feb 7, 2024

View reviewed changes

Yaxhveer added 3 commits February 7, 2024 22:24

changes updated

fa4a8e3

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

Merge branch 'main' into test-report-optional

be20fed

changes updated

26f55a9

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

Yaxhveer requested a review from PranshuSrivastava February 7, 2024 17:19

Merge branch 'main' into test-report-optional

fde1d52

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

removed extra logs

2177e52

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

Merge branch 'keploy:main' into test-report-optional

c9bd4e5

PranshuSrivastava approved these changes Feb 11, 2024

View reviewed changes

PranshuSrivastava merged commit e5e0872 into keploy:main Feb 11, 2024
13 checks passed

github-actions bot locked and limited conversation to collaborators Feb 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: added generateTestReport flag #1513

Feature: added generateTestReport flag #1513

Yaxhveer commented Feb 6, 2024

Yaxhveer commented Feb 6, 2024

gouravkrosx commented Feb 7, 2024 •

edited

Yaxhveer commented Feb 7, 2024

PranshuSrivastava Feb 7, 2024

Yaxhveer Feb 7, 2024

PranshuSrivastava Feb 7, 2024

Yaxhveer Feb 7, 2024

PranshuSrivastava Feb 7, 2024

Yaxhveer Feb 7, 2024

PranshuSrivastava Feb 7, 2024

Yaxhveer Feb 7, 2024

PranshuSrivastava Feb 8, 2024

Yaxhveer Feb 8, 2024 •

edited

PranshuSrivastava Feb 9, 2024

PranshuSrivastava commented Feb 9, 2024

Yaxhveer commented Feb 9, 2024

PranshuSrivastava left a comment

		}

		t.logger.Info("", zap.Any("keploy test and mock path", path), zap.Any("keploy testReport path", testReportPath))

		var ps *proxy.ProxySet

		defer pkg.DeleteTestReports(g.logger, generateTestReport)

Feature: added generateTestReport flag #1513

Feature: added generateTestReport flag #1513

Conversation

Yaxhveer commented Feb 6, 2024

Related Issue

Describe the changes you've made

Type of change

Please let us know if any test cases are added

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

Checklist:

Screenshots (if any)

Yaxhveer commented Feb 6, 2024

gouravkrosx commented Feb 7, 2024 • edited

Yaxhveer commented Feb 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yaxhveer Feb 8, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PranshuSrivastava commented Feb 9, 2024

Yaxhveer commented Feb 9, 2024

PranshuSrivastava left a comment

Choose a reason for hiding this comment

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

gouravkrosx commented Feb 7, 2024 •

edited

Yaxhveer Feb 8, 2024 •

edited