Skip to main content

Please review my fix for CR 6662107 ""

5 replies [Last post]
sergey_borodin
Offline
Joined: 2006-10-20
Points: 0

Please review fix.

diffs are:
https://jtharness.dev.java.net/source/browse/jtharness?rev=812&view=rev
https://jtharness.dev.java.net/source/browse/jtharness?rev=813&view=rev

Was decided, that the best way to solve this RFE is to make Documentation tab optional for test suite. This is done through FeatureManager settings, which has false as default value for SHOW_DOCS_FOR_TEST property.

If Documentation tab support is turned on in FeatureManager, behavoir will be the same as for other test tabs - if we select Documentation tab for test with enabled doc tab, and then switch to test with disabled one, tab will be displayed with empty content (in our case, ""), and its header will be grayed out.

Documentation tab for particular test becomes disabled, if respective method returns null instead of file list.

Thanks,
Sergey Borodin

Reply viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.
bkurotsu
Offline
Joined: 2004-12-13
Points: 0

My only problem with this behavior is that it transfers the problem to the documentation. Now the documentation must say that the tab may or may not be present.

Otherwise, as I understand it, I do like this solution better than continually hiding and unhiding the tab.

sergey_borodin
Offline
Joined: 2006-10-20
Points: 0

yes, Brian, I'm agree with you.

I've changed place, where decision is made, from TP_DocumentationSubpanel into TestPanel.

diff with previous commit is:
https://jtharness.dev.java.net/source/browse/jtharness?rev=832&view=rev

in this case TP_DocumentationSubpanel.java rolled back.

Thanks,
Sergey

bkurotsu
Offline
Joined: 2004-12-13
Points: 0

Hi Sergey - I don't think you understood what I said correctly.

I have no opinion whether your first or second attempt to fix was better. I only suggest that you optimize the number of calls to testSuite.getDocsForTest(td); It seems that the current implementation results in calling it twice every time the selected test is shown. Can we get this down to one call?

Brian

sergey_borodin
Offline
Joined: 2006-10-20
Points: 0

Oh, I see.

Yes, you're right, getDocsForTest() is called twice.

I prepared alternative fix then - could you please verify it?

diff:
https://jtharness.dev.java.net/source/browse/jtharness?rev=834&view=rev

One weakness of this variant is that we update Docs subpanel every time the test is changed, so no lazy update here, but I do not think it's problem for docs. I didn't found more elegant solution

Thanks,
Sergey

sergey_borodin
Offline
Joined: 2006-10-20
Points: 0

As I do not see any objections, I integrated this fix.

Final diff is:
https://jtharness.dev.java.net/source/browse/jtharness?rev=837&view=rev

Thanks,
Sergey