Turing integration tests should be moved to (or included in) Turing repo

[penelopeysm]

DynamicPPL contains a series of integration tests in the test/turing directory.

The way it's set up in CI, it fetches the most recent version of Turing, and if it's compatible with the version of DynamicPPL being tested, it runs the tests. If it's not compatible, it logs it and exits.

DynamicPPL.jl/.github/workflows/IntegrationTest.yml

Lines 35 to 51 in 18af48a

	- name: Load this and run the downstream tests
	shell: julia --color=yes --project=downstream {0}
	run: |
	using Pkg
	try
	# force it to use this PR's version of the package
	Pkg.develop(PackageSpec(path=".")) # resolver may fail with main deps
	Pkg.update()
	Pkg.test(julia_args=["--depwarn=no"]) # resolver may fail with test time deps
	catch err
	err isa Pkg.Resolve.ResolverError || rethrow()
	# If we can't resolve that means this is incompatible by SemVer and this is fine
	# It means we marked this as a breaking change, so we don't need to worry about
	# Mistakenly introducing a breaking change, as we have intentionally made one
	@info "Not compatible with this release. No problem." exception=err
	exit(0) # Exit immediately, as a success
	end

This unfortunately has the potential to cause breakage, as illustrated by this sequence of events with DynamicPPL 0.30:

1. DynamicPPL v0.30 was released.
   • This release contained a bug, FixedContext and ConditionedContext don't use the same varnames as tilde-pipeline #702, which would have been caught by the integration tests if they were run. However, because upstream Turing wasn't compatible with 0.30 at the time, the integration tests were not run.
2. Turing 0.35.2 was released. I fixed the tests that broke in the Turing test suite, but the DPPL integration tests didn't get run.
3. On the next PR to DynamicPPL, which was on something completely unrelated, the integration tests failed.
4. But we have already released Turing 0.35.2 which means that people out there can be using a bugged version where generated_quantities resamples values it shouldn't be resampling.

The solution to this, IMO, is to move the integration tests to the Turing.jl repo. That way, this chain of events would have been arrested at step 2. It's true that DynamicPPL itself would have a buggy release, but at least we wouldn't be letting ordinary Turing-users access it; they would have to be fiddling with internals to access a version that wasn't compatible with Turing.

More generally speaking, the integration tests get run on new patch releases but don't get run on new minor releases (because Turing's compat can't have been updated yet). This is a bit counterintuitive, because it's far more likely that a minor release breaks Turing rather than a patch release. I think it also leads to a false confidence when seeing the green tick.

[penelopeysm]

Or, maybe even better, factorise them out into a separate repo (or file). Retain the behaviour in DPPL in that we attempt to catch failures on patch version bumps, but also separately add them to Turing tests so that we catch minor version bumps.

[devmotion]

series of integration tests in the test/turing directory.

These are not run by the IntegrationTest workflow but a part of the DynamicPPL tests (

DynamicPPL.jl/test/runtests.jl

Lines 110 to 131 in 18af48a

	@testset "turing" begin
	try
	# activate separate test environment
	Pkg.activate(DIRECTORY_Turing_tests)
	Pkg.develop(PackageSpec(; path=DIRECTORY_DynamicPPL))
	Pkg.instantiate()
	# make sure that the new environment is considered `using` and `import` statements
	# (not added automatically on Julia 1.3, see e.g. PR #209)
	if !(joinpath(DIRECTORY_Turing_tests, "Project.toml") in Base.load_path())
	pushfirst!(LOAD_PATH, DIRECTORY_Turing_tests)
	end
	include(joinpath("turing", "runtests.jl"))
	catch err
	err isa Pkg.Resolve.ResolverError || rethrow()
	# If we can't resolve that means this is incompatible by SemVer and this is fine
	# It means we marked this as a breaking change, so we don't need to worry about
	# Mistakenly introducing a breaking change, as we have intentionally made one
	@info "Not compatible with this release. No problem." exception = err
	end
	end

). The IntegrationTest workflow runs the tests in Turing.jl with the current development version of DynamicPPL (if it is supported by Turing).

IMO step 1 is fine (of course, a bug-free release would be better). AFAICT the problem with step 2 was that the bug was only caught by additional tests in DynamicPPL (test/turing) that are not part of the Turing test suite. IMO these tests should just be moved to Turing.jl - if Turing.jl is compatible with the development version of DynamicPPL, these tests will still be run as part of the IntegrationTest workflow in DynamicPPL.

[penelopeysm]

TODO

• Add these tests to Turing.jl Add DynamicPPL integration tests Turing.jl#2393
• Remove these tests from DynamicPPL.jl (and corresponding machinery)

Stretch goal:

• move test_setval! to the DynamicPPL.TestUtils module so that we don't need to duplicate its definition in Turing tests