On Tue, Aug 16, 2011 at 10:15 AM, Brian Topping <[email protected]> wrote: > On Aug 15, 2011, at 10:21 PM, Martijn Dashorst wrote: > >> I'm not sure why all this effort is going into something that works >> really well for us, and has worked for years. I don't see any benefit >> from using an obscure maven plugin, while we have a test framework >> setup that works. Sounds like make work to me. > > Not at all! I never noticed it before, that my projects have a transitive > dependency on JUnit. It's a dependency that I don't see other projects I > work with depending on, and it's probably a dependency that will never be > made into a bundle. It's only being used for Assert in a few places, as I > wrote earlier. This is one of the features that I liked the most in my early days in Wicket - its testing functionality. Many other frameworks don't provide such and thus don't require testing frameworks like JUnit. > > Assertions are for testing invariants (assumptions that must be true in order > for the code to continue), and it is assumed that situations where the > invariants fail have been caught as coding errors during development and > testing. Letting JUnit Assert throw unspecified RuntimeExceptions obfuscates > what's actually going on. It's just a bad idea for a lot of reasons. > > Regarding the obscurity of the Maven plugin, it's been in Brix for years and > we've had no problem with it at all. In fact, it's a lot more convenient > because it will can be re-invoked to automatically add proper headers to > missing files after they are found. > > It was mentioned during the week that Jeremy was working on changing things > to use this plugin and ran out of time. It seemed like an idea that had > already been vetted by a number of folks, so I went that direction. I can't > imagine that he would have undertaken that work as a committer if there was a > chance it would be rejected like that. Jeremy? > >> As far as the junit dependency: wicket tester requires it, and we ship >> wicket tester in core. I don't want wicket tester going to a different >> JAR as I consider it an essential tool in Wicket development. I remember some people wanted to make JUnit optional so they can choose between JUnit or TestNG and the solution was that all JUnit dependencies were moved out of BaseWicketTester to WicketTester and these users could make their own impl with TestNG. It seems this was broken in 1.5 because BaseWicketTester again have some JUnit imports. > > Of course it is, but does it ever get used at runtime? I'm trying to be > constructive here, don't you believe test code should be in test modules and > not bundled in to production artifacts? How does one additional test scope > dependency sound so difficult or complex?
WicketTester is mostly used for tests but some people use it to generate mail templates in production/runtime too. So it is not that optional. > >> Especially since OSGi should have some way of excluding junit should >> you want to do that. I don't have an objection to marking junit >> optional though. > > JUnit can't be optional or excluded if there is code that depends on it. > OSGi is still Java. :-) Of course it can be optional. Unless you use WicketTester and Co. in runtime there is no need to register and start a bundle for JUnit. > >> >> I don't see an argument that we should make life for 99% of wicket >> users more difficult, as well as our own lives, for just one OSGi >> dependency. > > I guess I don't understand the difficulty. If it has to do with an extra > dependency, I'd merely ask if 99% of Wicket users use Hibernate, shouldn't > Hibernate be a default dependency? For Maven (Ivy/Gradle/...) users this is not a big deal. The problem comes for the rest (build systems w/o dependency management) - these users start to ask questions in the mailing lists why something is not there. IMO these users are the beginners, they don't think in Object Oriented way, nor in modular, they expect everything to be in one big .jar... > > The goal is to help improve Wicket, a framework that has been important to a > lot of us for years. There's also a lot of new interest in the OSGi > community around Wicket, and a lot of smart people there that would be great > to have associated with any project. I would *never* advocate that Wicket > should become harder to use for J2EE users, the group of people that would be > using OSGi is not nearly large enough to warrant that. And if I was just > saying that, I wouldn't have spent days digging into resolving this issue. > But if Wicket remains hard to use for OSGi users, it's a bad sign in a lot of > ways. > > Everything happens for a reason in the end, right? :-) For me extracting WicketTester and related classes in separate module is a plus. I'm just not happy that we try to do that in the last? RC of 1.5. About the Maven plugin - I have no experience with it so I can't say anything about it. All I know is that the current unit tests work and there are no open issues about them. And I don't see any problem they can cause to the OSGi related improvements. > >> >> Martijn >> >> On Mon, Aug 15, 2011 at 9:39 AM, Brian Topping <[email protected]> wrote: >>> Thanks for the feedback, Martin. >>> >>> I also ran into the issues around .js testing and the dynamic .html >>> expected output. In the latter case, I didn't dive deeper to realize >>> that's what I was looking at, but I didn't pursue it either. >>> >>> Regarding the test jar(s), there is also >>> o.a.w.extensions.ajax.markup.html.AjaxLazyLoadPanelTester and may be others >>> I haven't gotten to yet. It seems like the right thing to do is to create >>> a wicket-extensions-test (which has a dependency on wicket-test). This is >>> not an ideal pattern, but part of the issue with the module structure of >>> extensions not being in core. And if wicket-extensions were a standalone >>> project somewhere else on the internet, we'd still need a >>> wicket-extensions-test, so I don't think this is a degenerate case. >>> >>> I'll build out the latter with two test modules and we can take a look at >>> it. >>> >>> Back to the topic of the license work, is there anything I can do to help >>> prove / disprove the work I've done? I'm happy to do deeper research >>> anywhere it's necessary, the bulk of the work on that is already done so it >>> doesn't make sense to slow down now. >>> >>> On Aug 15, 2011, at 12:10 PM, Martin Grigorov wrote: >>> >>>> Hi Brian, >>>> >>>> By unknown rules I meant whether there are some expectations by >>>> official Apache tools. I remember a tool named RAT which was used in >>>> 1.2.x and early 1.3.x releases but I know nothing more about it. >>>> >>>> .html files in test/java can be dynamic, i.e. WicketTestCase can save >>>> the output for a component/page and use it as "expected" result for >>>> future runs. >>>> So it is not possible to put the licence in all .html files. >>>> >>>> Another exception is usage of third party .js/.css files in >>>> wicket-examples - e.g. JQuery, Prototype.js >>>> >>>> I also like the idea of wicket-test.jar but a problem I see is that >>>> wicket-extensions also have **TesterHelper for LazyLoadPanel, and >>>> maybe wicket-test module will have to depend on -extensions ... >>>> >>>> Recently I tried to revive Matej's work on Wicket-Ajax NG and I tried >>>> to put all Ajax stuff in wicket-ajax.jar. It almost works but there >>>> were some problems which I have written somewhere but this is off >>>> topic here. >>>> >>>> On Mon, Aug 15, 2011 at 6:13 PM, Brian Topping <[email protected]> >>>> wrote: >>>>> Hi Martin, comments inline. >>>>> >>>>> On Aug 15, 2011, at 3:50 AM, Martin Grigorov wrote: >>>>> >>>>>> Is it possible to configure the plugin to fail on unknown files ? >>>>> >>>>> There are no limitations of what we can do, even if we need to fork the >>>>> mycila plugin to do it. :-) But if you mean "can the plugin as written >>>>> deal with unknown files, the only way to do that would be to include >>>>> everything (<include>**</include>) and then write an exclude that matches >>>>> every file that should not be included. This will catch new files that >>>>> are unknown in the way you mentioned. I didn't come to believe that the >>>>> old system did this though, just that it handled the six or so types >>>>> (java, js, xml, vm, css, and a couple others I don't remember ATM). >>>>> >>>>>> E.g. currently it excludes 'target/' folder and checks files with >>>>>> extensions .java, .xml, .html, .properties, .js, .css and .vm. Is it >>>>>> possible to add some global excludes for images and configure it to >>>>>> fail the verification if an unknown file is found. I.e. if there is >>>>>> src/main/java/org/apache/wicket/SomeFile.blah and it is not explicitly >>>>>> excluded then fail >>>>> >>>>> My experience with the old method of using unit tests to test that the >>>>> source code had headers was not an exact process either. I had seen many >>>>> files that did not have headers for no apparent reason. What I tried to >>>>> do was find a "best fit" to the existing rules. With as many files as >>>>> there are in the system, it would require an automated test rig to ensure >>>>> that every file was being managed exactly like the old system did, and >>>>> given that the old system had holes caught on visual inspection, it >>>>> didn't seem that was time well-spent. >>>>> >>>>>> >>>>>> Additionally can you add some comments around why some resources are >>>>>> excluded ? >>>>>> I just saw wicket-core/pom.xml has: >>>>>> <plugin> >>>>>> 83 >>>>>> + <groupId>com.mycila.maven-license-plugin</groupId> >>>>>> 84 >>>>>> + <artifactId>maven-license-plugin</artifactId> >>>>>> 85 >>>>>> + <configuration> >>>>>> 86 >>>>>> + >>>>>> <header>${project.parent.basedir}/licenses/asf-license-header.txt</header> >>>>>> 87 >>>>>> + <excludes> >>>>>> 88 >>>>>> + >>>>>> <exclude>src/main/java/org/apache/wicket/**/*.properties</exclude> >>>>>> >>>>>> and I wonder why line 88 is there. >>>>> >>>>> Yes, line 88 is probably incorrect. I'll take a look at it. >>>>> >>>>>> I know current JUnit based tests also have some exclusions but I'm not >>>>>> sure whether they have explanation "why". >>>>> >>>>> The issue is with test data. For instance, a .html file in the test tree >>>>> may not actually be used in the compilation of a test, it may be just for >>>>> comparison that WicketTester created the final output that is correct. >>>>> Adding a header to a file like that would cause the comparison (and the >>>>> test) to fail. >>>>> >>>>> We could add that header to both the test input and test output, but I >>>>> was trying to take the path of least impact. >>>>> >>>>>> >>>>>> On Mon, Aug 15, 2011 at 10:06 AM, Martin Grigorov <[email protected]> >>>>>> wrote: >>>>>>> Hi, >>>>>>> >>>>>>> The change at >>>>>>> https://github.com/topping/wicket/commit/8f08c2082f90fcd54a7e4734b892d13e610a47bf >>>>>>> looks OK to me but I'm not quite familiar with the unit test based >>>>>>> solution (the current one). I'm not sure whether there are some >>>>>>> (unknown for me) rules like specific formatting which may become >>>>>>> broken now. >>>>> >>>>> This doesn't seem to be the case. I'm committed to supporting the change >>>>> that I made here though, so whatever minor changes are necessary, I'm >>>>> happy to have issues assigned to me. >>>>> >>>>>>> >>>>>>> I guess Andreas' solution will be needed at least for wicket-core >>>>>>> because as I said (Base)WicketTester depends on JUnit - both compile >>>>>>> and runtime. >>>>> >>>>> Yes, the best practice in this case is to have a test module containing >>>>> test code that has a runtime dependency on a library such as JUnit. >>>>> There was a huge amount of changes to get this in, and I overlooked that >>>>> o.a.w.util.tester is in core. Having it remain that way would cause the >>>>> entire body of work to be pretty, but not very useful to it's original >>>>> end. >>>>> >>>>> Are there practical reasons that o.a.w.util.tester is in core? >>>>> >>>>> <snip/> >>>>> >>>>> Cheers, Brian >>>> >>>> >>>> >>>> -- >>>> Martin Grigorov >>>> jWeekend >>>> Training, Consulting, Development >>>> http://jWeekend.com >>>> >>> >>> >> >> >> >> -- >> Become a Wicket expert, learn from the best: http://wicketinaction.com >> > > -- Martin Grigorov jWeekend Training, Consulting, Development http://jWeekend.com
