I’ve uploaded a new webrev: http://cr.openjdk.java.net/~hannesw/8176453/webrev.02/
This allows to set the build/test artefact directory using the working.dir system property: ant -Dworking.dir=… By default the current working directory is used as I don’t think there’s a safe default location that works cross platform. It is also possible to set the lib.dir property to another directory containing selenium-server-standalone-3.13.0.jar so it will not attempt the download. However the download is still attempted if the file is not present. The only problem with adding (lots of) classes/packages/modules is repository bloat. As an alternative, it would be quite easy to point the tests to other, existing classes belonging to the neighbouring tests. Hannes > Am 14.08.2018 um 01:43 schrieb Jonathan Gibbons <[email protected]>: > > Hannes, > > This looks like a good start. > > It would be better if you did not always download/install Selenium. It > should be possible to run the tests with the Ant file configured to point to > a local already-installed version of Selenium. If nothing else, this helps > avoid problems related to proxies. Yes, I know that you set > skipexisting="true" but you'll still download Selenium each time in a fresh > repo or after "make clean" etc. > > Without looking at it in great depth, it looks like the test might write > files in the main test/ directory (or some subdirectory thereof). That is > generally bad practice. Think of the src/ and test/ dirs in the repo as > read-only. You should be able to configure where any files will be written. > > How easy/practical will it be to update the test code (that you run through > javadoc) to have many modules/packages/classes so that Javadoc Search has > lots to work on? i.e. lots of test cases within it. > > -- Jon > > > On 8/9/18 9:14 AM, Hannes Wallnöfer wrote: >> Sundar, Jon: thanks for the reviews, and sorry for the late reply (I was on >> vacation last week). >> >> As suggested I went ahead and wrote some Selenium tests for this and the >> previous search bug I fixed, along with a simple Ant build file to run them. >> I included this in the webrev in >> test/langtools/jdk/javadoc/doclet/seleniumTests - not sure if this a proper >> location. It includes its own test sources for generating API docs. >> >> http://cr.openjdk.java.net/~hannesw/8176453/webrev.01/ >> >> Instructions for running the tests are included as comment in the main test >> class (JavaDocSearchTest.java). Unfortunately browser drivers still require >> manual configuration as these are highly platform specific. >> >> Some additional comments: >> >> - I’m pretty sure that ‚&‘ will not occur in search results as type >> parameters are not displayed and wildcards cannot have multiple bounds. We >> could still escape it to be sure, but I thought it was more important to >> keep the overhead to a minimum and so kept it at just ‚<‚ and ‚>'. >> >> - The ant task to download the Selenium jar file uses the HTTP URL instead >> of secure HTTPS. This is because HTTPS fails for me, probably because I did >> not include closed sources. >> >> Let me know if you think this is the way to go. >> >> Hannes >> >> >> >>> Am 31.07.2018 um 00:39 schrieb Jonathan Gibbons >>> <[email protected]>: >>> >>> While parameterized types typically just contain < and >, the bounds may >>> sometimes include &, so I tend to agree with Sundar that if you need to >>> escape < > you probably need to escape & as well. >>> >>> Even if we can't automate tests (yet?) I think it would be good for you to >>> set up test cases for manual tests that allow us to check the behavior on >>> the OS and browser of our choice. Simply asking us to test this is not >>> enough. >>> >>> If you can find test cases that leverage the latest JDK API, then you could >>> point at that. Otherwise you could construct test cases and post them near >>> the webrev on your account as cr.ojn. Sometimes I will just build the >>> latest JDK API and post that on cr.ojn. >>> >>> -- Jon >>> >>> >>> On 07/30/2018 07:19 AM, Sundararajan Athijegannathan wrote: >>>> Don't you also need ".replace(/&/g,'&')"? >>>> >>>> PS. Isn't there a standard way to escape HTML in JS? >>>> >>>> >>>> -Sundar >>>> >>>> On 27/07/18, 7:51 PM, Hannes Wallnöfer wrote: >>>>> Please review and test this patch to escape < and > characters in search >>>>> results to HTML < and > entities. >>>>> >>>>> Issue: >>>>> https://bugs.openjdk.java.net/browse/JDK-8176453 >>>>> >>>>> Webrev: >>>>> http://cr.openjdk.java.net/~hannesw/8176453/webrev.00/ >>>>> >>>>> >>>>> This fixes rendering of search results for search terms containing >>>>> generics such as „map<str“. >>>>> >>>>> Thanks, >>>>> Hannes >>>>> >>>>> >
