Fix format of a downloaded json file #1306
Conversation
Katarzyna-B
requested review from
everett980,
rubenvp8510 and
tiffon
as code owners
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1306 +/- ##
==========================================
+ Coverage 95.51% 95.60% +0.09%
==========================================
Files 244 244
Lines 7651 7653 +2
Branches 2006 2006
==========================================
+ Hits 7308 7317 +9
+ Misses 343 336 -7
... and 1 file with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
yurishkuro
changed the title
| @@ -113,7 +113,7 @@ export class UnconnectedSearchResults extends React.PureComponent<SearchResultsP | |||
| }; | |||
|
|
|||
| onDownloadResultsClicked = () => { | |||
| const file = new Blob([JSON.stringify(this.props.rawTraces)], { type: 'application/json' }); | |||
| const file = new Blob([`{"data":${JSON.stringify(this.props.rawTraces)}}`], { type: 'application/json' }); | |||
There was a problem hiding this comment.
Is it possible to add a test for this kind of behavior?
- Resolves jaegertracing#663 - fix format of a downloaded json file ## Short description of the changes - fix a format of a downloaded file to be able to use for uploading back into the UI Signed-off-by: katarzyna <[email protected]>
Katarzyna-B
force-pushed
the
batch_download_of_traces_fix_format
branch
from
8ec45d7 to
04334d7
Compare
| const view = 'traces'; | ||
| wrapper.setProps({ location: { search: `${otherSearch}&${searchParam}=${view}` } }); | ||
|
|
||
| const download = wrapper.find(DownloadResults).prop('onDownloadResultsClicked'); | ||
| download(); | ||
| expect(URL.createObjectURL).toBeCalledTimes(1); | ||
| expect(URL.createObjectURL).toBeCalledWith(file); | ||
| expect(file.text).toBe(content); |
There was a problem hiding this comment.
So this tests that the downloaded content is in this specific format, not that the specific format is what the rest of the UI expects. Besides this simple assert, I would prefer to call into the functionality that's normally invoked when Upload is executed, i.e. something that parses the uploaded results and transforms them into search results. Then we can assert that the search results are what we expect given this rawTraces input. This way the test will ensure that the download format is always what is expected by the upload, right now it does not really do that.
There was a problem hiding this comment.
I got your point.
I need to check/learn how I can do it in the best way.
I'm not sure if I have enough time to write such kind of the integration test before my vacation.
In the worst case I will try to do it in the middle of the April.
There was a problem hiding this comment.
to clarify, I don't mean some elaborate integration test, simply calling an "upload" function that must exist somewhere in the code already, to validate the results / format.
There was a problem hiding this comment.
I pushed my changes, where I refactor a little bit the code and I used now readJsonFile in a test. This method is used by the upload too.
If it is something still missing I can do it first in two weeks.
Have a great day.
Katarzyna
- Resolves jaegertracing#663 - fix format of a downloaded json file ## Short description of the changes - fix a format of a downloaded file format to be able to use for uploading back into the UI Signed-off-by: katarzyna <[email protected]>
- Part of jaegertracing#663 - fix format of a downloaded json file ## Short description of the changes - fix a format of a downloaded file to be able to use for uploading back into the UI Signed-off-by: katarzyna <[email protected]> --------- Signed-off-by: katarzyna <[email protected]>
- Part of jaegertracing#663 - fix format of a downloaded json file ## Short description of the changes - fix a format of a downloaded file to be able to use for uploading back into the UI Signed-off-by: katarzyna <[email protected]> --------- Signed-off-by: katarzyna <[email protected]> Signed-off-by: RAMU MANAM <[email protected]>
Short description of the changes
Signed-off-by: katarzyna [email protected]