On Fri, Dec 4, 2020 at 3:29 PM Brian Hulette <[email protected]> wrote:
> > > On Wed, Dec 2, 2020 at 5:50 PM Brian Hulette <[email protected]> wrote: > >> I guess another question I should ask - is :test supposed to only run >> unit tests? I've been assuming so since many modules have separate >> :integrationTest tasks for *IT tests. >> >> On Wed, Dec 2, 2020 at 4:15 PM Kyle Weaver <[email protected]> wrote: >> >>> > DicomIOTest, FhirIOTest, >>> HL7v2IOTest, org.apache.beam.sdk.extensions.ml.*IT >>> >>> Looking only at the former three tests, I don't see any reason they >>> can't mock their API clients, especially since all they expect the server >>> to do is send back an error. >>> >> >> Fair point, that wouldn't be *too* much trouble. More than just >> re-classifying them as integration tests though :) >> > Can we write the tests to be agnostic as to whether the service is mocked, faked, or real? Then we can run them in various modes. I'm a strong believer in "fake don't mock" to the extent that I think mocking these might be counterproductive. On a case-by-case basis, perhaps one can write a tiny in-memory version that has the functionality needed rather than exact scripted responses. This will interact well with the idea of making the test unaware of whether it is running against the real service or not. This way we can run a fast (or at least hermetic) version as a unit test and once in a while sanity check it against the real service. (In a perfect world, owners of services would always ship a local testing fake and own keeping it up to date... testcontainers is one approach but also in-memory fakes are great) Kenn > > This seems like something that is easy to get wrong without some >>> automation to help. Could we run the :test targets on Jenkins using the >>> sandbox command or docker to block network access? >>> >>> That's a great idea. Are we planning on integrating the "standardized >>> developer build environment" mentioned in the original post into our CI >>> somehow? >>> >> >> I was thinking it could be good to use it in CI somehow to make sure it >> doesn't get out of date, but all I had in mind was running some minimal set >> of tasks. Using it in this way would obviously be even better. >> > > I filed https://issues.apache.org/jira/browse/BEAM-11404 to track this > idea. > > >> >> >>> >>> On Wed, Dec 2, 2020 at 4:03 PM Andrew Pilloud <[email protected]> >>> wrote: >>> >>>> We have a large number of tests that run pipelines on the Direct Runner >>>> or various local runners, but don't require network access, so I don't >>>> think the distinction is clear. I do agree that requiring a remote service >>>> falls on the integration test side. >>>> >>>> This seems like something that is easy to get wrong without some >>>> automation to help. Could we run the :test targets on Jenkins using the >>>> sandbox command or docker to block network access? >>>> >>>> On Wed, Dec 2, 2020 at 3:38 PM Brian Hulette <[email protected]> >>>> wrote: >>>> >>>>> Sorry I should've included the list of tests here. So far we've run >>>>> into: >>>>> DicomIOTest, FhirIOTest, >>>>> HL7v2IOTest, org.apache.beam.sdk.extensions.ml.*IT >>>>> >>>>> Note the latter are called IT, but that package's build.gradle has a >>>>> line to scoop ITs into the :test task (addressing in [1]). >>>>> >>>>> All of these tests are actually running pipelines so I think they'd be >>>>> difficult to mock. >>>>> >>>>> [1] https://github.com/apache/beam/pull/13444 >>>>> >>>>> On Wed, Dec 2, 2020 at 3:28 PM Kyle Weaver <[email protected]> >>>>> wrote: >>>>> >>>>>> > Should we (do we) require unit tests to be hermetic? >>>>>> >>>>>> We should. Unit tests are hermetic by definition. That begs the >>>>>> definition of hermetic, but clearly the internet is not. >>>>>> >>>>>> > Personally I think these tests should be classified as integration >>>>>> tests (renamed to *IT, and run with the :integrationTest task) >>>>>> >>>>>> I'm not sure which tests you're talking about, but it may be better >>>>>> to make them hermetic through mocking, depending on the intent of the >>>>>> test. >>>>>> >>>>>> On Wed, Dec 2, 2020 at 1:22 PM Brian Hulette <[email protected]> >>>>>> wrote: >>>>>> >>>>>>> I've been working with Niels Basjes on a standardized developer >>>>>>> build environment that can run `./gradlew check` [1]. We've run into >>>>>>> issues >>>>>>> because several Java unit tests (class *Test, run with :test) are not >>>>>>> hermetic. They fail unless the environment they're running in has >>>>>>> access to >>>>>>> the internet, and is authenticated to GCP with access to certain >>>>>>> resources. >>>>>>> Of course the former isn't typically a blocker, but the latter certainly >>>>>>> can be. >>>>>>> >>>>>>> Personally I think these tests should be classified as integration >>>>>>> tests (renamed to *IT, and run with the :integrationTest task), but I >>>>>>> realized I don't know if we have a formal definition for what should be >>>>>>> a >>>>>>> unit test vs an integration test. Should we (do we) require unit tests >>>>>>> to >>>>>>> be hermetic? >>>>>>> >>>>>>> Brian >>>>>>> >>>>>>> [1] https://github.com/apache/beam/pull/13308 >>>>>>> >>>>>>
