Test expectations, at least. A ton of tests started failing when I added headers to those files. I don't know if there are functional problems too. I'd ideally like to get that all straightened out in the next release, if we can survive as is for 1.6.1.
A. On Jun 11, 2013 7:51 AM, "Tom White" <[email protected]> wrote: > That makes a lot of sense - thanks for the explanation, Ignasi. > > There are other files in the test directory that should have headers > added though, like the log4j.xml files. It would be easier to filter > if the test expectation files were in an identifiable directory, e.g. > src/test/resources/expected. That way when other files are added to > the test hierarchy (that are not expectation files), then they won't > be filtered inadvertently. This is probably not absolutely required > for the first release though. > > Regarding the META-INF/services files - there are lots of these files > in main that you should be able to add headers to using # as the > comment character, see > > https://github.com/apache/whirr/blob/trunk/core/src/main/resources/META-INF/services/org.apache.whirr.ClusterController > . > Is there any reason why headers can't be added to the test ones too? > > Cheers, > Tom > > On Mon, Jun 10, 2013 at 7:26 PM, Ignasi <[email protected]> wrote: > > Thanks for checking Tom! > > > > Please, apologize me if this has already been answered, but I am > > really confused and I still don't get what should be done with the > > headers in test files (and it is important to have it clear because it > > affects what and how we can perform tests). > > > > I really think that XML files in src/test/resources should not have > > test headers. As said in other threads, those XML files are used just > > to verify the output of the cloud APIs. In clouds we have two kinds of > > tests: > > > > * Expect tests (offline): These tests just mock the cloud apis, and we > > use them to properly check that we are generating the requests as > > expected, and we are properly parsing the responses. The XML/JSON > > files in src/test/resources are just a copy of the real responses the > > cloud apis return, and we use them to build the mock responses as > > close to the reality as possible. > > * Live tests (online): These tests actually execute the requests > > against the cloud apis, and check the behavior against real cloud > > providers. > > > > Having this clear, I think those XML/JSON files fail into the > > "non-creative" category (they are just a copies of real http response > > bodies). Furthermore, since the real cloud apis don't return an HTTP > > response body with license headers, our "response templates" shouldn't > > have them either. That's why we think it is important to keep those > > files without the license headers; adding them will make it impossible > > to properly add live tests that check the response bodies. > > > > > > Does this make sense? If it does, can we keep JSON/XML files in the > > src/test/resources directory without the license headers? > > > > > > Thanks for your patience, mentors! > > > > > > Ignasi > > > > > > On 10 June 2013 19:56, Andrew Bayer <[email protected]> wrote: > >> And there are a good number of tests failing because XML files aren't > >> expected to have headers. I'll see what I can do. > >> > >> A. > >> > >> On Mon, Jun 10, 2013 at 10:54 AM, Andrew Bayer <[email protected] > >wrote: > >> > >>> Also, if those don't full under the category of "non-creative", I don't > >>> know what does. =) > >>> > >>> A. > >>> > >>> > >>> On Mon, Jun 10, 2013 at 10:48 AM, Andrew Bayer <[email protected] > >wrote: > >>> > >>>> Ok, the META-INF/services stuff will need at least test changes to > work > >>>> right with license headers added, if not full blown code changes, so > that > >>>> won't be happening. > >>>> > >>>> A. > >>>> > >>>> > >>>> On Mon, Jun 10, 2013 at 9:41 AM, Andrew Bayer <[email protected] > >wrote: > >>>> > >>>>> Will do. > >>>>> > >>>>> A. > >>>>> > >>>>> > >>>>> On Mon, Jun 10, 2013 at 9:21 AM, Tom White <[email protected]> wrote: > >>>>> > >>>>>> I ran through basic checks on the source packages (checksums, > >>>>>> signatures, license headers, included binary files, LICENSE, NOTICE, > >>>>>> DISCLAIMER), and generally they look good. In addition to the points > >>>>>> made on > >>>>>> > https://wiki.apache.org/jclouds/1.6.1%20Incubating%20Release%20Issues, > >>>>>> the only thing I would add is that RAT flags up lots of missing > >>>>>> license headers. Many of them are JSON which can be excluded since > >>>>>> JSON doesn't support comments (as discussed above), or tests, but > the > >>>>>> META-INF/services, YAML, XML, and properties files should have > license > >>>>>> headers added for RC3 if possible. > >>>>>> > >>>>>> Cheers, > >>>>>> Tom > >>>>>> > >>>>>> On Thu, Jun 6, 2013 at 4:03 PM, Ignasi <[email protected]> > >>>>>> wrote: > >>>>>> > David, have you had a chance to take a look at my comments about > the > >>>>>> > json stuff? http://markmail.org/message/pk7efsmc6ewmake7 > >>>>>> > > >>>>>> > I've also noted my doubts on the wiki so the conclusions for each > one > >>>>>> > can be reflected there. > >>>>>> > > >>>>>> > On 5 June 2013 23:07, Andrew Bayer <[email protected]> > wrote: > >>>>>> >> Dealt with for RC3, and noted on > >>>>>> >> > >>>>>> > https://wiki.apache.org/jclouds/1.6.1%20Incubating%20Release%20Issues > >>>>>> >> > >>>>>> >> A. > >>>>>> >> > >>>>>> >> On Wed, Jun 5, 2013 at 1:40 PM, David Nalley <[email protected]> > wrote: > >>>>>> >> > >>>>>> >>> On Mon, Jun 3, 2013 at 7:58 PM, Andrew Bayer < > >>>>>> [email protected]> > >>>>>> >>> wrote: > >>>>>> >>> > I'm extending the vote another 24 hours for our mentors to > >>>>>> respond. > >>>>>> >>> > > >>>>>> >>> > A. > >>>>>> >>> > > >>>>>> >>> > > >>>>>> >>> > >>>>>> >>> jclouds-karaf has both LICENSE and LICENSE.txt and I'd argue > that > >>>>>> >>> LICENSE.txt should be purged. Aside from that it looks in good > >>>>>> shape. > >>>>>> >>> > >>>>>> >>> --David > >>>>>> >>> > >>>>>> > >>>>> > >>>>> > >>>> > >>> >
