On Wed, Dec 2, 2020 at 5:50 PM Brian Hulette <bhule...@google.com> 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 <kcwea...@google.com> 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 :) > > >> >> > 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 <apill...@google.com> >> 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 <bhule...@google.com> >>> 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 <kcwea...@google.com> 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 <bhule...@google.com> >>>>> 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 >>>>>> >>>>>
