Bug 57578 - HTTPRequest : When md5 is checked Embedded Resources from HTML files should be disabled
Summary: HTTPRequest : When md5 is checked Embedded Resources from HTML files should b...
Status: NEW
Alias: None
Product: JMeter
Classification: Unclassified
Component: HTTP (show other bugs)
Version: 2.12
Hardware: All All
: P2 enhancement (vote)
Target Milestone: ---
Assignee: JMeter issues mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-02-12 22:27 UTC by Philippe Mouawad
Modified: 2015-02-15 17:39 UTC (History)
1 user (show)



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Philippe Mouawad 2015-02-12 22:27:46 UTC
When MD5 is checked as response is not stored, no parsing can occur.
So checking anything in "Embedded Resources from HTML files" panel won't have any effect.
I suggest we disable checkbox "Retrieve All embedded resources" when "Save response as MD5 Hash" is checked.

By the way, it seems HttpTestSampleGui uselessly implemented ItemListener as none of the addItemListeners in classes uses it. So I suggest we drop it adding this to "Incompatible changes" as subclasses may be impacted.
Comment 1 Sebb 2015-02-12 23:31:20 UTC
(In reply to Philippe Mouawad from comment #0)
> When MD5 is checked as response is not stored, no parsing can occur.

It could occur, but does not currently.

> So checking anything in "Embedded Resources from HTML files" panel won't
> have any effect.
> I suggest we disable checkbox "Retrieve All embedded resources" when "Save
> response as MD5 Hash" is checked.

If the user decides they then want to retrieve the embedded resources, won't this make it rather awkward for the user? They will have to disable Md5 first.
And why should MD5 disable embedded but not the other way round?

I think it would be better to just make a note in the manual.

It might be OK to deselect embedded (rather than disabling it) when selecting MD5 -- and vice versa.

Alternatively, maybe the code _should_ still parse the responses, but replace the sample data with the MD5 when it is no longer needed. This will require more storage whilst the sample is being processed. The code should only store the full response if it needs to be parsed - i.e. images etc. would be converted to MD5 directly. It might be possible to parse the response as it is read, rather than waiting for it to be fully loaded first.

However, this would change the test plan behaviour. There would need to be some way to disable it.

> By the way, it seems HttpTestSampleGui uselessly implemented ItemListener as
> none of the addItemListeners in classes uses it. So I suggest we drop it
> adding this to "Incompatible changes" as subclasses may be impacted.

That belongs in a separate Bugzilla issue.
Comment 2 Philippe Mouawad 2015-02-15 17:39:54 UTC
(In reply to Sebb from comment #1)
> (In reply to Philippe Mouawad from comment #0)
> > When MD5 is checked as response is not stored, no parsing can occur.
> 
> It could occur, but does not currently.
> 
> > So checking anything in "Embedded Resources from HTML files" panel won't
> > have any effect.
> > I suggest we disable checkbox "Retrieve All embedded resources" when "Save
> > response as MD5 Hash" is checked.
> 
> If the user decides they then want to retrieve the embedded resources, won't
> this make it rather awkward for the user? They will have to disable Md5
> first.
> And why should MD5 disable embedded but not the other way round?
> 
> I think it would be better to just make a note in the manual.
Ok, why not. I created this bugzilla as I was not aware of this behaviour. I still believe sometimes it's better that UI makes the right decision which in this case it to indicate in a way that embedded resources won't be downloaded.

> 
> It might be OK to deselect embedded (rather than disabling it) when
> selecting MD5 -- and vice versa.
> 
> Alternatively, maybe the code _should_ still parse the responses, but
> replace the sample data with the MD5 when it is no longer needed. This will
> require more storage whilst the sample is being processed. The code should
> only store the full response if it needs to be parsed - i.e. images etc.
> would be converted to MD5 directly. It might be possible to parse the
> response as it is read, rather than waiting for it to be fully loaded first.

Seems better to me indeed.
> 
> However, this would change the test plan behaviour. There would need to be
> some way to disable it.
Why not when it's enabled.
> 
> > By the way, it seems HttpTestSampleGui uselessly implemented ItemListener as
> > none of the addItemListeners in classes uses it. So I suggest we drop it
> > adding this to "Incompatible changes" as subclasses may be impacted.
> 
> That belongs in a separate Bugzilla issue.

Done in Bug 57586