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. > > 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 >>>>> >>>>
