URL encode test result files

[Swiddis]

Description

Following up on #4569: URL encode the filenames in the test manifest file to better support tools scanning these files for links.

Issues Resolved

Closes #4569.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.40%. Comparing base (97770ce) to head (4bc14e3).
Report is 13 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4574      +/-   ##
==========================================
+ Coverage   92.17%   92.40%   +0.23%     
==========================================
  Files         192      193       +1     
  Lines        6297     6317      +20     
==========================================
+ Hits         5804     5837      +33     
+ Misses        493      480      -13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[zelinh]

Thanks for the PR @Swiddis.
I am aware of this issue and this might cause confusion; but I believe the link should be accessible if you include all of the URL address with spaces.

One thing I'm worried about this PR is that our implementation should also be compatible with local directory address when we pass in local base_path so replacing space here may not be working for local test record.

[Swiddis]

Hm, good point on local test suites... I could try introducing a check for if base_path starts with http(s):// if that would suffice? Or maybe more generally [scheme]://.

I'm aware that the link is accessible if you copy the address with spaces -- it's mostly annoying with some text editors when copying directly from yaml, since they typically won't respect the yaml unquoted-string whitespace conventions and this ends up in needing to find and delete stray spaces.

[zelinh]

Hm, good point on local test suites... I could try introducing a check for if base_path starts with http(s):// if that would suffice? Or maybe more generally [scheme]://.

I'm aware that the link is accessible if you copy the address with spaces -- it's mostly annoying with some text editors when copying directly from yaml, since they typically won't respect the yaml unquoted-string whitespace conventions and this ends up in needing to find and delete stray spaces.

Yeah I think you could have another check to see if base_path starts with https:// as what we did here.

opensearch-build/src/test_workflow/test_recorder/test_recorder.py

Lines 40 to 44 in b86cf75

	if base_path.startswith("https://"):
	file_path = "/".join([base_path.strip("/"), "test-results", str(self.test_run_id), self.test_type, component_name, component_test_config])
	else:
	file_path = self._create_base_folder_structure(component_name, component_test_config)
	return file_path

[Swiddis]

Updated the logic and associated test cases

[peterzhuamazon]

Does it make sense to change on line 40 instead?

https://github.com/opensearch-project/opensearch-build/blob/main/src/test_workflow/test_recorder/test_recorder.py#L40

[peterzhuamazon]

Does it make sense to change on line 40 instead?

https://github.com/opensearch-project/opensearch-build/blob/main/src/test_workflow/test_recorder/test_recorder.py#L40

Ignore it, I think I didnt realize the file value is appended later on.

[peterzhuamazon]

Thanks.

[zelinh]

LGTM! Thanks.