feat: support write the report to a HTTP server #367

hahahashen · 2024-04-15T12:07:30Z

No description provided.

CLAassistant · 2024-04-15T12:07:37Z

All committers have signed the CLA.

hahahashen · 2024-04-15T12:13:50Z

我看了SonarCloud Code Analysis check失败的原因，一个是测试代码pwd疑似硬编码，一个是测试代码中代码重复率较高，不知道这两点需要改吗

pkg/runner/writer_http.go

LinuxSuRen

您还需要根据提示签一下 CLA

@hahahashen 请注意下这里的 comment

#367 (comment)

pkg/runner/writer_http.go

LinuxSuRen · 2024-04-16T02:47:53Z

pkg/runner/writer_http.go

+	if w.templateFile == "" {
+		// 使用默认模板将数据序列化为 JSON 格式
+		payload, err := json.Marshal(result)
+		if err != nil {
+			log.Fatal("Error marshaling JSON:", err)
+			return err
+		}
+		req, err = http.NewRequest(w.requestMethod, url, bytes.NewBuffer(payload))
+		if err != nil {
+			log.Println("error when NewRequest", err)
+			return err
+		}


这段逻辑基本是对着的，但是下面的部分有点问题。模板文件应该是特定格式的文件，例如：JSON、YAML 等，这也是常见的 Payload 格式；另外，还需要添加响应的 Content-type Header 描述 Payload 的格式。

我理解是，要提供一下JSON、XML、YAML等常见payload格式供调用者选择，并正确设置Content-type Header ？而不是像我这里这样直接就使用json了

不需要提供那么多的选择，你搜索下 Go Template，只需要提供一个设置模板的参数即可。也就是你去调用 Go Template 去把模板渲染下作为 HTTP 的请求体（也就是 Payload ）。至于格式的话，感觉可以写死 JSON（毕竟大部分 API 都才用这种格式），后续有其他诉求了再增加不同格式的支持也可以。

LinuxSuRen · 2024-04-16T02:49:12Z

pkg/runner/writer_http.go

+func (w *httpResultWriter) getTemplateFileRequest(url string) (req *http.Request, err error) {
+
+	// 打开要发送的文件
+	file, err := os.Open(w.templateFile)


对于默认的模板，你可以采用 Golang Embed 的方式来获取模板文件内容，不需要再读取文件了。当然，如果用户传递的是一个本地文件，可以采用这个方式。

如果你想做的更加方便一点，甚至可以从 URL 里读取一个模板文件。

我本来理解是调用者传递一个本地文件给remote server
是需要再加上识别该文件如果是go模板文件则使用Golang Embed 的方式来获取模板文件内容来进行优化吗？如果是其他格式的文件则维持原逻辑？

Golang Embed 是为了读取内置的模板。统一用 Go Template 来替代 json.Marshal(result)，这样可扩展性就很好。

pkg/runner/writer_http_test.go

LinuxSuRen · 2024-04-16T02:54:32Z

我看了SonarCloud Code Analysis check失败的原因，一个是测试代码pwd疑似硬编码，一个是测试代码中代码重复率较高，不知道这两点需要改吗

当业务逻辑代码 review 通过后，我们可以综合考虑来看是否要忽略 Sonar 给出的建议。你可以参考 Sonar 给出的建议来试着完善、优化代码。通过 ChatGPT 或类似产品，也可以帮忙优化代码。一般来说，如果单元测试难度很大可以降低覆盖率。

LinuxSuRen

你还需要考虑下如何启动这个功能，请参考下面的代码：

api-testing/cmd/run.go

Line 134 in e05ff4b

switch o.report {

另外，麻烦把手动测试的截图或者相关记录也贴以下，方便 review

可以考虑在下面的位置增加一个参数 --report-template 接收来自用户给定的模板文件。

api-testing/cmd/run.go

Line 111 in e05ff4b

    
           flags.BoolVarP(&opt.reportIgnore, "report-ignore", "", false, "Indicate if ignore the report output")

pkg/runner/writer_http.go

LinuxSuRen · 2024-04-17T01:37:49Z

pkg/runner/writer_http.go

+
+func (w *httpResultWriter) getTemplateFileRequest(url string, result []ReportResult) (req *http.Request, err error) {
+
+	tmplData, err := content.ReadFile("writer_templates/" + w.templateFile.filename)


用户传过来的模板文件就没有办法通过 Embed 的方式来读取了，Embed 会把相应的文件在构建的时候一并放在二进制文件中。

LinuxSuRen · 2024-04-17T01:39:13Z

pkg/runner/writer_http.go

+		payload, err := json.Marshal(result)
+		if err != nil {
+			log.Fatal("Error marshaling JSON:", err)
+			return err
+		}


按照我的理解，你可以在这里读取 Embed 的 JSON 格式的模板文件，并渲染。因此，判断的逻辑可以是如果用户给了模板文件，就从本地文件中读取，如果没有，就从 Embed 中读取。然后，所有的情况都通过 Go Template 来渲染。

如果用户没有给模板文件，就从embed里面读取我自己事先编好的模板文件吗？

是的，这样流程、逻辑比较简单和统一

hahahashen · 2024-04-17T11:34:38Z

你还需要考虑下如何启动这个功能，请参考下面的代码：

api-testing/cmd/run.go

Line 134 in e05ff4b

switch o.report {

另外，麻烦把手动测试的截图或者相关记录也贴以下，方便 review

可以考虑在下面的位置增加一个参数 --report-template 接收来自用户给定的模板文件。

api-testing/cmd/run.go

Line 111 in e05ff4b

flags.BoolVarP(&opt.reportIgnore, "report-ignore", "", false, "Indicate if ignore the report output")

url以及请求类型是post、get还是别的类型以及请求参数是不是也需要用户指定呀

hahahashen · 2024-04-17T12:44:52Z

先按照我自己的理解写了一版，后续再完善之后我再合并一下commit
测试我首先go test进行测试，测试通过了

在编译完之后，我起了一个server监听8080端口，然后使用 ./api-testing run --report http命令进行测试，测试结果如下：

LinuxSuRen · 2024-04-17T12:54:10Z

cmd/run.go

@@ -153,6 +153,9 @@ func (o *runOption) preRunE(cmd *cobra.Command, args []string) (err error) {
 	case "github":
 		o.githubReportOption.ReportFile = o.reportFile
 		o.reportWriter, err = runner.NewGithubPRCommentWriter(o.githubReportOption)
+	case "http":
+		templateOption := runner.NewTemplateOption("./pkg/runner/writer_templates/example.tpl", "json")


NewTemplateOption 的第一个参数应该是用户给的，你可以添加一个如下参数：

--report-template

而且，默认值是空的。

LinuxSuRen · 2024-04-17T12:57:26Z

cmd/run.go

@@ -153,6 +153,9 @@ func (o *runOption) preRunE(cmd *cobra.Command, args []string) (err error) {
 	case "github":
 		o.githubReportOption.ReportFile = o.reportFile
 		o.reportWriter, err = runner.NewGithubPRCommentWriter(o.githubReportOption)
+	case "http":
+		templateOption := runner.NewTemplateOption("./pkg/runner/writer_templates/example.tpl", "json")
+		o.reportWriter = runner.NewHTTPResultWriter("POST", "http://localhost:8080", nil, templateOption)


你可以使用这个常量：http.MethodPost替换 POST

http://localhost:8080 不能写死，需要用户传递，参数可以是 --report-dest

嗯，是的

其实不用生成一个文件，把用户传递的模板文件在内存中渲染了，直接作为 HTTP Payload 发出去就可以了，并不需要持久化的。另外，JSON 是非常流行的、主流的格式，个人感觉目前可以只支持 JSON 格式。

LinuxSuRen · 2024-04-17T12:59:07Z

pkg/runner/writer_http.go

+			url = fmt.Sprintf("%s&%s=%s", url, key, value)
+		}
+	}
+	log.Println(url)


We could provide more context. For example: log.Println("will send report to:" + url)

LinuxSuRen · 2024-04-17T12:59:40Z

pkg/runner/writer_http.go

+	var tmpl *template.Template
+	if w.templateFile == nil {
+		// use the default template file to serialize the data to JSON format
+		tmpl, err = template.New("example template").Parse(exampleTemplate)


Please give it a meaning full name. For example: template.New("HTTP report template")

LinuxSuRen · 2024-04-17T13:00:25Z

pkg/runner/writer_http.go

+	} else {
+		content, err := os.ReadFile(w.templateFile.filename)
+		if err != nil {
+			fmt.Println("Error reading file:", err)


Please use log instead of fmt.Println

LinuxSuRen · 2024-04-17T13:00:53Z

pkg/runner/writer_http.go

+			return err
+		}
+
+		tmpl, err = template.New("mytemplate").Parse(string(content))


Please use a meaning full name.

LinuxSuRen · 2024-04-17T13:02:21Z

pkg/runner/writer_http.go

+}
+
+//go:embed writer_templates/example.tpl
+var exampleTemplate string


How about change it to defaultTemplate.

hahahashen · 2024-04-22T02:51:46Z

按照comment进行了相应的修改，后续如果没有更改我再合并一下commit
go test测试通过

./api-testing run --report http --report-template ./pkg/runner/writer_templates/example.tpl --report-dest http://127.0.0.1:8080
命令测试通过

LinuxSuRen · 2024-04-22T04:46:45Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

shenmengju seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

This is required as well.

LinuxSuRen · 2024-04-22T04:50:54Z

I'm wondering if you set the git user.email same to the GitHub email setting.

If you set it correctly, you can see the avatar and click the link.

Quality Gate failed

Failed conditions
3 Security Hotspots
13.2% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

hahahashen · 2024-04-22T08:26:28Z

You can squash all commits into one with current account.

done

LinuxSuRen

Good job. Thank you for your patience and understanding.

LinuxSuRen · 2024-04-22T08:28:55Z

So happly to have as the 16th contributor.

* chore(deps): update jenkins/jenkins docker tag to v2.420 * Update app version --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: github-action update-app-version <githubaction@githubaction.com>

hahahashen commented Apr 15, 2024

View reviewed changes

pkg/runner/writer_http.go Show resolved Hide resolved

LinuxSuRen requested changes Apr 16, 2024

View reviewed changes

LinuxSuRen changed the title ~~feat: add httpResultWriter~~ feat: support write the report to a HTTP server Apr 16, 2024

LinuxSuRen added the enhancement New feature or request label Apr 16, 2024

LinuxSuRen linked an issue Apr 16, 2024 that may be closed by this pull request

Support sending test report to a HTTP server #91

Closed

hahahashen force-pushed the feat/addHttpResultWriter branch 2 times, most recently from a175594 to bf2e023 Compare April 16, 2024 10:06

hahahashen requested a review from LinuxSuRen April 16, 2024 10:28

LinuxSuRen requested changes Apr 17, 2024

View reviewed changes

hahahashen force-pushed the feat/addHttpResultWriter branch 2 times, most recently from dabbf93 to afc65d4 Compare April 22, 2024 06:25

hahahashen requested a review from LinuxSuRen April 22, 2024 06:34

hahahashen force-pushed the feat/addHttpResultWriter branch from afc65d4 to 6dc057a Compare April 22, 2024 08:20

add httpResultWriter

f2a0dab

hahahashen force-pushed the feat/addHttpResultWriter branch from 6dc057a to f2a0dab Compare April 22, 2024 08:25

LinuxSuRen approved these changes Apr 22, 2024

View reviewed changes

LinuxSuRen merged commit ea911ae into LinuxSuRen:master Apr 22, 2024

hahahashen deleted the feat/addHttpResultWriter branch April 23, 2024 06:07

LinuxSuRen added the ospp 开源之夏 https://summer-ospp.ac.cn/ label May 17, 2024


		func (w httpResultWriter) getTemplateFileRequest(url string, result []ReportResult) (req http.Request, err error) {

		tmplData, err := content.ReadFile("writer_templates/" + w.templateFile.filename)

feat: support write the report to a HTTP server #367

feat: support write the report to a HTTP server #367

Uh oh!

Conversation

hahahashen commented Apr 15, 2024

Uh oh!

CLAassistant commented Apr 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hahahashen commented Apr 15, 2024

Uh oh!

Uh oh!

LinuxSuRen left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hahahashen Apr 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

LinuxSuRen commented Apr 16, 2024

Uh oh!

LinuxSuRen left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hahahashen commented Apr 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hahahashen commented Apr 17, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hahahashen commented Apr 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LinuxSuRen commented Apr 22, 2024

Uh oh!

LinuxSuRen commented Apr 22, 2024

CLAassistant commented Apr 15, 2024 •

edited

Loading

LinuxSuRen left a comment •

edited

Loading

hahahashen Apr 16, 2024 •

edited

Loading

LinuxSuRen left a comment •

edited

Loading

hahahashen commented Apr 17, 2024 •

edited

Loading

hahahashen commented Apr 22, 2024 •

edited

Loading