Add app-testing contract RFC - #153
Conversation
Types answer what kind of test; lifecycle position is exposed as ATS_UPGRADE_STAGE=pre|post, consistent with how tests gate on all other environment properties. Keeps the type set identical to the published ATS contract.
Setup and teardown are app-specific like assertions and duplicate across harnesses the same way. Conventional executables get the same env as tests; the portable boundary stays the app cluster's KUBECONFIG, harness-native hooks remain for MC-side work.
Canonical prefix is harness-neutral; runners dual-export the legacy ATS_ names so nothing breaks. ATS_APP_CONFIG_FILE_PATH becomes APP_TEST_VALUES_FILE and ATS_CLUSTER_VERSION becomes APP_TEST_KUBERNETES_VERSION.
It drives the harnesses' existing upgrade primitives and doubles as a lint against typo'd upgrade tags.
- split test types into orthogonal depth (smoke/functional) and flow (upgrade) axes; define how a multi-tagged test executes - require committed lockfiles and frozen/offline installs - define cluster types by capability and warn against using them as a harness proxy; define external and the neither/empty-directory cases - specify hooks as out-of-process executables, exempt from one-language - add converse upgrade lint and optional expectedTypes collection check - add contractVersion, a conformance suite, and a named steward - note the two-Go-module layout and go.work
…back-latency handling Problem section now leads with the deliberate fast-CI-on-kind vs nightly-e2e-on-WC split and the two-idiom authoring cost that suppressed adoption, rather than test duplication. Adds a cross-runner parity assertion to the conformance suite (settled semantics: Helm-installed vs App-CR-deployed), a Cadence and feedback latency section with an on-demand /run escape hatch for nightly-only cloud paths, and a single-runner alternative.
The two-axes/compose taxonomy did not match either harness. ATS's UpgradeTestScenario runs only the upgrade StepType; atf's upgrade suite runs its test function once after upgrading. Neither re-runs smoke and functional around the upgrade. Makes upgrade a third peer type (smoke, functional, upgrade) that runs once after the upgrade, moves pre-upgrade state seeding into an imperative pre-upgrade hook, drops APP_TEST_UPGRADE_STAGE and the pre/post double run. Hooks section now states hooks do side-effecting work and tests assert, which is what lets the upgrade flow seed without re-running a suite.
Post-only dropped the baseline. Running the upgrade-typed tests on the old version before upgrading is what makes a post failure attributable to the upgrade rather than a pre-existing break, and it is cheap. Restores pre+post for the upgrade type (with APP_TEST_UPGRADE_STAGE) while keeping the two real fixes: upgrade is a peer type, and smoke/functional never run in the upgrade flow. ATS is unchanged (it already runs pre+post); atf's convention-runner gains the pre run. pre-upgrade hook stays for pure side-effect seeding.
Convention discovery of tests/app/hooks/{setup,pre-upgrade,teardown} is
the portable zero-wiring default every runner must implement; harnesses
also keep their existing hook flags, so existing repos need no immediate
move and harness-specific points stay available. If a flag and a
convention hook target the same contract point the runner fails fast, so
migration is drop-file-remove-flag in one change, not a silent double-run.
Adds a per-harness mapping table and records that ATS needs a new
pre-deploy point for setup (its --app-tests-pre-hook fires after deploy).
Rewrites the doc in plainer language, cuts repetition and the argues-with-a-reviewer asides, and stops claiming the ATS_ dual export is free (it is deprecated, dropped at the next contract version). No decisions changed by the rewrite. Drops config key upgrade: true. The upgrade flow is now inferred from the presence of upgrade-typed tests, matching presence-is-the-opt-in used everywhere else. Removes the two cross-lints that only existed to guard the flag; expectedTypes still makes a type mandatory.
Version negotiation is overkill for two in-house runners that both track this doc and are gated by the same conformance suite. The contract evolves by changing the suite in the same PR; a lagging runner is a bug, not a version mismatch to arbitrate. Removes the config key and the versioning paragraph, renames the section to Conformance and ownership, and reworks the ATS_ deprecation to not reference a next version.
- correct broken TEST_CONTRACT.md link (master -> main) - fix atf kubeconfig call: framework.MC().GetClusterKubeConfig - pin settled on IsReleaseReady (all workload kinds) not IsDeploymentReady - add APP_TEST_CAPABILITIES; gate on capability, never cluster type/harness - correct upgrade-stage mapping (ATS_EXTRA_UPGRADE_TEST_STAGE, value change) - correct offline install guarantee (GOPROXY=off, uv --offline; atf too) - reflect app-test-suite#675 (ATS runs on a provided cluster)
Fold in the test-time controllers contract so charts that need a running
controller (Flux, Argo, External Secrets) to reconcile the CRs they create
can declare them once and have any conforming harness bootstrap them.
- New "Prerequisite controllers" section: declarative, named, versioned
controllers the runner installs before deploy (the declarative sibling of
the setup hook); semver via Masterminds/semver v3; per-harness values files
listed via a `harness[]` list (name as a field, not a map key); significant
install order; reuse-if-present with a version check; install-only lifecycle.
- Shared config moves to `.apptest/config.yaml` (code in tests/app/,
declarations in .apptest/); controllers + their values files live there.
- Runner guarantees: controllers bootstrapped and ready before the setup hook
and the app deploy.
- Implementation + conformance: both runners parse and bootstrap controllers;
the conformance fixture declares one and the parity check covers it.
- Adjust the rejected `values:{ats,e2e}` alternative to allow the bounded
`harness[]` exception for controller values.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sync the in-repo test-time controllers specs with giantswarm/rfc#153 (the app-testing contract), where the harness-neutral contract now lives: - test-time-controllers-contract.md: reduced to a summary + pointer to the RFC (the authoritative harness-neutral home), avoiding a drifting duplicate. - ats-test-time-controllers.md: per-controller values now use the RFC's harness[] list (name + valuesFile) instead of the config.ats/config.atf map; cross-references point at the RFC. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Pushed a commit ( What it adds:
Three of the edits are design changes to review, not just additions:
Happy to split any of these out or adjust if you'd rather keep the shared file under |
| #### Capabilities | ||
|
|
||
| The cluster intended for PR testing is `kind`. As a simple cluster instance, it lacks features like cloud |
There was a problem hiding this comment.
I don't think that's true, ATF is not running kind
There was a problem hiding this comment.
Thanks, that was meant for ats only, will fix
| | `APP_TEST_RELEASE_NAME` | yes | Helm release name of the app under test | | ||
| | `APP_TEST_RELEASE_NAMESPACE` | yes | namespace the app is deployed into | | ||
| | `APP_TEST_CHART_VERSION` | yes | version of the chart under test | | ||
| | `APP_TEST_CLUSTER_TYPE` | yes | type of the cluster the app runs on - a label: `kind` (local single-node), `capi` (a CAPI workload cluster), or `external` (a cluster the runner did not provision). Describes shape, not capability; gate on `APP_TEST_CAPABILITIES` instead | |
There was a problem hiding this comment.
For ATS all clusters are external so do we really need APP_TEST_CLUSTER_TYPE and APP_TEST_KUBERNETES_VERSION?
There was a problem hiding this comment.
Good question... My idea here was to just give the test devs the visibility into what they can expect of the type of the cluster, like "can we deploy and test HA mode", which might not make sense on kind. That info won't come from just kubernetes cluster version.
There was a problem hiding this comment.
Should this not be a capabilities then?
| To avoid forcing a specific test framework or technology on test authors, we decided to make a convention that | ||
| tests are executed as a separate process by the test toolkits. The only requirements are that: | ||
|
|
||
| - for each test suite, we group all the tests into: |
There was a problem hiding this comment.
I don't see how this maps with atf model:
- We can run multiple suites, each on a different WC in parallel
- A suite can be standard or upgrade
- Each suite is run per provider (or the provided defined for that suite)
Could you provide an example?
There was a problem hiding this comment.
The idea is that ats and atf will converge here (and changes in both are needed): the mapping between test types is, I think, functional-standard, upgrade is the same, and smoke is empty for atf right now. As for "where does it run", I'm not sure what you mean: this can run wherever atf can start it, it's the toolkit's config and the CI/CD process that should make this decision, not how the tests are executed. Does that make sense?
| - a single test can be of multiple types, for example a test that checks if the app is deployed and running | ||
| can be both smoke and functional; a test that checks if the main page of an app loads can (and probably | ||
| should) be functional and upgrade. | ||
| - tests need to be runnable with `go test` (golang) or `pytest` (python), and are using test filtering to run |
There was a problem hiding this comment.
atf works with ginkgo. the pipeline assumes certain things that wouldn't be there if the tests were not structured following that framework. (e.g. printing results)
There was a problem hiding this comment.
True today, but it's a small entrypoint change, not an architectural one. Two things make it cheap:
- Ginkgo specs run under plain go test, so existing suites don't get rewritten. The contract explicitly allows Ginkgo (see Alternatives), it just stops requiring it.
- The reporting path is JUnit XML, which isn't Ginkgo-specific: gotestsum --junitfile produces it from any go test run, and pytest emits it natively (--junitxml, ATS already relies on this). display-test-results keeps working unchanged.
| - tests need to be runnable with `go test` (golang) or `pytest` (python), and are using test filtering to run | ||
| only the tests of a specific type (smoke, functional, upgrade). The test filtering is done with build tags | ||
| (golang) or markers (python). | ||
| - all the information about the test environmenrt is passed to the tests via environment variables, and the |
There was a problem hiding this comment.
atf uses several config files to define multiple aspects of the configuration. why limiting it to envvars?
also, in the case of ats, how could I configure the kind cluster where the test is executed?
There was a problem hiding this comment.
It's not limited to envvars, it's "config file first, then env vars overrides". The whole point here is that it's much easier to set env vars in CI environment than it is to dynamically generate a config file.
As for ats: ats will no longer manager kind cluster lifecycle, the CI process will.
|
|
||
| # TODO: just a config proopsal, discuss | ||
|
|
||
| ## Alternatives considered |
There was a problem hiding this comment.
Have you consider ditching ats and adding a kind or vcluster provider in atf? TBH adopting the same framework is way better than trying to make them match the contract.
There was a problem hiding this comment.
I'd like to enumerate some advantages:
- atf is more flexible
- test metrics are unified
- results checks are uniform
- everything runs on the same infrastructure, debugging gets easier
- integration out of the box with agent analysis tooling that tenet is working on
There was a problem hiding this comment.
atf's install path assumes App Platform or Flux HelmReleases and an MC. Local PR testing has no MC. A kind provider fixes provisioning but you'd still have to make the install layer optional/pluggable and strip the MC dependency from the core, at which point you've rewritten ATS inside atf so we are trying to not to that at all.
There was a problem hiding this comment.
path assumes App Platform or Flux HelmReleases and an MC
this is an implementation detail, in the test you don't need to know about the MC nor the deployment process.
Local PR testing has no MC
true, but from the PoV of a test, you don't need to know about the MC.
you'd still have to make the install layer optional/pluggable and strip the MC dependency from the core
why? this RFC claims that the goal is to gain speed, so being the MC where the cluster is running is not relevant.
at which point you've rewritten ATS inside atf so we are trying to not to that at all.
why? keeping 2 separated toolkits or frameworks makes every new feature and improvement cost twice.
There was a problem hiding this comment.
path assumes App Platform or Flux HelmReleases and an MC
this is an implementation detail, in the test you don't need to know about the MC nor the deployment process.
This is not true. For observability, we inject the installation name so we need to know about the MC but the main issue with the MC is that this is an invariant of atf and app testing actually boots up a full MC plus deploys apps which is just unbearably slow for testing a helm chart change.
On top of that, we want to support installng our charts (support only models) on non giant swarm cluster and this is not possible with ATF, because MCs is actually needed there
you'd still have to make the install layer optional/pluggable and strip the MC dependency from the core
why? this RFC claims that the goal is to gain speed, so being the MC where the cluster is running is not relevant.
Because the market is mostly using Argo and in Bumblebee we need to support both flux and argo.
at which point you've rewritten ATS inside atf so we are trying to not to that at all.
why? keeping 2 separated toolkits or frameworks makes every new feature and improvement cost twice.
Not specifically as long as both respect the contract. ATS is more generic/flexible in nature that ATF with enforces a giantswarm MC and a giantswarm cluster where kind and so on is not possible at all. Also, you would lose python support which some teams like to have and that would required quite a big migration effort
There was a problem hiding this comment.
This is not true. For observability, we inject the installation name so we need to know about the MC but the main issue with the MC is that this is an invariant of atf and app testing actually boots up a full MC plus deploys apps which is just unbearably slow for testing a helm chart change.
On top of that, we want to support installng our charts (support only models) on non giant swarm cluster and this is not possible with ATF, because MCs is actually needed there
So, if I read this correctly you have 2 use-cases:
- Testing mimir/loki as an integrated part of our platform.
- Testing mimir/loki as a standalone app.
1 is only covered by the atf with the long test: creating the ephemeral MC and a WC. This is not solved by ats.
2 is what you expect to solve with ats. That's also possible in atf, as you can install the app in an WC. We do that for envoy-gateway and works fine. You just need design the test that way, which is the default use-case for all apps. This is what I say that it could be speed up adding a kind/vcluster to atf.
Because the market is mostly using Argo and in Bumblebee we need to support both flux and argo.
I don't follow what the deployment method has to do with this RFC.
Not specifically as long as both respect the contract.
Contract has nothing to do with features that happen under the hood. Metrics or agent based reports/analysis are custom for each toolkit. If you want to have them in both, you need to double the implementation.
ATS is more generic/flexible in nature that ATF with enforces a giantswarm MC and a giantswarm cluster where kind and so on is not possible at all.
in my exp, ats is not more flexible. It has the limitations of running on CircleCI. Again, in atf, from the test perspective you don't need to know that the WC you are working with is a giantswarm one (yeah sure, you'll need to comply with some policies, but these are good practices anyway)
Also, you would lose python support which some teams like to have and that would required quite a big migration effort
I can't comment on team preferences, but from my exp the migration effort is not that big.
There was a problem hiding this comment.
@mcharriere you touched on the very good point where this whole discussion between Quentin and me actually started. We have opted for "2 tools, 1 way to write tests" because:
atfseems to be very opinionated (for a good reason) about the environment where it runs the tests: it has to be a real GS WC/MC cluster. Adding there "the minimal cluster" option seemed like addingif (minimal) {a} else {b}all over the place (no-go) or doing a really heavy refactoring, that will effectively backportatson top of it, as one of its working modes,- With the above in scope and having in mind that both tools are already used and tested in their current forms, we proposed the current solution as the way to skip the heavy refactoring of both tools and tests and then getting some maturity and stability out of the new tool; starting from scratch today, I would agree this should be a single tool, but the price of unifying them now seems high compared to the benefits.
This for sure isn't a perfect solution, but we preferred this over the alternative.
| | Legacy | Canonical | Note | | ||
| | ----------------------------------------------------------------- | ----------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | `ATS_TEST_TYPE` | `APP_TEST_TYPE` | straight rename | | ||
| | `ATS_APP_CONFIG_FILE_PATH` | `APP_TEST_VALUES_FILE` | straight rename | |
There was a problem hiding this comment.
Value files in the name might be confusing. Does it mean the config file below - that can also contain a list of Helm value files - or just the Helm value file? There actually can be more than one of the latter, so I kind of assume it the config file.
There was a problem hiding this comment.
I also found this name confusing
There was a problem hiding this comment.
Naming is hard... do you have better ideas? Is APP_TEST_CHART_VALUES_FILES better? Yes, it's for passing values files (plural) to the installed helm chart.
| | --------------------------------------------------------------- | ----------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | `KUBECONFIG` | yes | kubeconfig of the cluster the app is deployed on (never the MC) | | ||
| | `APP_TEST_TYPE` | yes | the type currently being run (`smoke`, `functional`, `upgrade`) | | ||
| | `APP_TEST_TOOLKIT` | yes | The name of the toolkit running the tests (`ats` or `atf`) | |
There was a problem hiding this comment.
I get it that it can be useful to push this down, but assuming the tests should be able to run regardless of environment - sure, Helm values and App Test config files can make some differences - I mostly see this as an abuse surface to make the tests different based on where they run. Not having this could make it easier to enforce the test really run anywhere. Wdyt?
There was a problem hiding this comment.
I agree, the difference should the capabilities a cluster has, not the toolkit running the tests.
| 1. Test toolkit detects that tests are present in `tests/app/`. | ||
| 1. Test toolkit installs software dependencies for the test suite (go or python) using the lockfile in | ||
| `tests/app/`. | ||
| 1. Test toolkit prepares the cluster used for testing (installs tools or dependencies it needs to execute |
There was a problem hiding this comment.
If possible, lets do it in a way that the tools are "installed" standalone to avoid polluting local machines especially and that the frameworks can easily use them instead of global ones.
One use case I have in mind is konfigure-operator that relies on Flux - well actually only source-controller - to fetch the source (tho it is extensible). So we need a way to install complex pre requisites and some of these, e.g. Flux I might not want to reinstall between runs, this this is more for the setup part. What I mean here is that now ATS installs Flux as an App CR. In these frameworks I assume its rather Helm directly that should do, so we might need a binary for that - or use a pure python (dunno if it exists) or the go client directly.
There was a problem hiding this comment.
Scenarios like this ones will be supported by an extension of this spec that we have in mind - but I think it's not worth it to discuss more features before we even approve the general idea. But I totally see your problem and we have already discussed it.
| flow: | ||
|
|
||
| 1. Test toolkit detects that tests are present in `tests/app/`. | ||
| 1. Test toolkit installs software dependencies for the test suite (go or python) using the lockfile in |
There was a problem hiding this comment.
With python sometimes you need certain libs, binaries, header files or even a compiler to install certain packages. Might make sense to have a hook before this too. Problem can be especially local runs (ATS), where the mode of getting such dependencies greatly vary between the host OS.
There was a problem hiding this comment.
I think this should be provided by the env, like during the CI setup process. Otherwise, we need a way for the toolkits to install dependencies, potentially system-wide, which means root access is needed. And we still probably need to make some assumptions about where the tool is run, because that will depend on the distro and such. CI should have this specific information and can install what's needed without enforcing the same approach on other CI environments.
There was a problem hiding this comment.
Makes sense. Just maybe it should be stated that double book keeping is required for CI runner and for whatever runs ATF (+ local). It might be an implantation detail how exactly, but a significant point and worth a bit of thoughts how this will be kept up to date.
Its also true however that the specific repos tests might need these so the repos set them up. But should / is there / can we make an interface for this? I am not sure we can, but then for each repo the maintainer must maintain it 3 times (local, ATS, ATF setup) and the tools must be able to call them in a specific manner, no?
I mean for local, devs can call it. For CI devs can put it into the CIrcleCI config maybe (tho then it could be better unified)? For ATS I dont know, depends on how it is ran.
stone-z
left a comment
stone-z
left a comment
There was a problem hiding this comment.
I'd argue atf and ats serve different purposes and should be decomposed instead of contractually aligned. Because we have unfortunately designed many things around MCs, there must be some awareness of that concept in e2e testing. That's 90% of what atf does now, and that won't go away any time soon. However, the actual test execution portion could be extracted entirely from atf and made to be reusable in CI. Then atf could invoke that thing for WC tests and again for MC tests.
I'm not opposed to trying this to make things better, I just don't think it's solving the right problem.
FWIW I echo Mati and think it makes more sense to just use one tool.
Do I understand correctly that the result of this is that I can write ats tests, and those tests will be run first in kind on CircleCI and then those same tests will also be run during e2e?
If so, why would I bother writing atf tests that only work in one place?
| Tests live in one directory: `tests/app/`. Any harness that deploys the app runs that directory the same way. | ||
| An empty `tests/app/` (no module, no project) isn't an opt-in and is skipped. | ||
|
|
||
| It's either one Go module or one Python project, not both. The runner picks the executor from what's there: |
There was a problem hiding this comment.
Does this mean we can't have some tests written in Go and some in Python for a given project?
There was a problem hiding this comment.
Yeah, currently we assumed it doesn't make sense to combine python and go tests in a single project, as that makes code sharing between test scenarios impossible. Do you have a use case for that?
There was a problem hiding this comment.
I was just clarifying. I don't necessarily want to have both. I'd prefer to write tests in Go, but it's more important to me that I can run tests everywhere, so if that means Python I'll do that.
Not supporting both in parallel just reinforces my top-level comment.
| type. | ||
| 1. `post-run` hook runs for `type` tests (if present). | ||
| 1. If there are `upgrade` tests: | ||
| 1. If the version under test (new) is already installed in the cluster, it is uninstalled. |
There was a problem hiding this comment.
This makes the test non-representative. The cluster must be created using the previous version for upgrade tests.
We commonly need to hand-fix cases where CRDs have changed and resources didn't upgrade cleanly for various reasons. Applying the new version of the application first guarantees that the new CRDs are present when the cluster and resources are created, which is not representative of a real update.
There was a problem hiding this comment.
Good point. Still, cluster lifecycle management is on purpose out of scope here. This should be solved in CI, with a flow like:
- create test cluster
- run only smoke anddd functional tests
- recreate test cluster from scratch
- run only the upgrade test
I think this is a common scenrio and as such it should be available and implemented in our CI/CD.
There was a problem hiding this comment.
Well, you're including the hooks and test flow as part of the RFC. So if it's something to be addressed, I think it should be, or if the test sequencing is something that is planned to be flexible and managed separately, then maybe it should be removed?
| type. | ||
| 1. `post-run` hook runs for `type` tests (if present). | ||
| 1. If there are `upgrade` tests: | ||
| 1. If the version under test (new) is already installed in the cluster, it is uninstalled. |
There was a problem hiding this comment.
Additional question -
I understand "upgrade" in the ats sense to mean "installing a new version of the app, within a cluster which remains at a fixed version".
How should I understand "upgrade" in an atf sense?
Because some of our apps are release defaults and some not, I would like an "upgrade" option that supports "install version 1 of the app into cluster version X, then upgrade the cluster to version Y"
There was a problem hiding this comment.
Again, a pefectly valid scenario, which we find our of scope here, as we try to keep cluster lifecycle separate. On a high level, this can again be a CI pipeline with high level flow like:
- deploy cluster v.X
- start tests with a test toolkit
- upgrade cluster to v.X+1
- start the same tests again using the toolkit
There was a problem hiding this comment.
Same comment here - the RFC describes the hooks that are possible, so since cluster lifecycle tests are relevant, it would be good to commit to some support for them in the RFC.
Or, if the RFC is intended to still be higher-level, then the implementation details don't belong here.
| | Legacy | Canonical | Note | | ||
| | ----------------------------------------------------------------- | ----------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | `ATS_TEST_TYPE` | `APP_TEST_TYPE` | straight rename | | ||
| | `ATS_APP_CONFIG_FILE_PATH` | `APP_TEST_VALUES_FILE` | straight rename | |
There was a problem hiding this comment.
I also found this name confusing
| will probably be able to deliver some of these capabilities. To cope with these differences, tests that need | ||
| those capabilities should gate on the `APP_TEST_CAPABILITIES`. | ||
|
|
||
| We define the following capabilities, which a toolkit might set in `APP_TEST_CAPABILITIES`: |
There was a problem hiding this comment.
Just a suggestion, not objecting, but I suspect it will be difficult for you to manage capabilities over time like this. People will use and abuse them in unexpected ways, and will need new ones for weird niches.
Rather than try to anticipate what the capability variants might be, an option might be making people be specific about what their test requires (e.g. by detecting a CRD, probing for a service).
Maybe there are KaaS tests where this makes more sense.
longer:
For cases I would like to test, capabilities seem like a way of declaring a dependency on an unspecific service, but the test probably requires (directly or indirectly) something specific.
For example, I want to test autoscaling of my app. That doesn't make sense in kind, but the autoscaling implementation already depends on the presence of HPA/VPA APIs. I could gate the test based on the CRDs.
Same with metrics. I would want to test that some metrics are collected from my application, but in order to do that I'd need to be able to create the Pod/ServiceMonitor (not in kind by default) and then I'd need to know where to get the metrics from. I could gate the test based on whether the CRDs are there and whether you passed me a metrics endpoint to hit (*).
In both cases, an autoscaling or metrics capability would just be masking what I really needed, and KaaS is going to think autoscaling means Karpenter and/or cluster-autoscaler anyway.
* the metrics example also gets into MC visibility and external "fixtures" which I don't see represented here yet.
There was a problem hiding this comment.
I see your point, but I'm not sure if gating on CRDs is really better. Let's say I have a chart that is using flux CRs internally: OCIRepository, HelmRelease and Kustomization. Should I check and gate on all 3 of them? What if I need 5? Or should I check just one and assume the rest is available? I would prefer to check that "capabilities" has "flux". But I can also imagine that I might be testing with a dependency that introduced a new CR, like MutatingPolicy replacing ClusterPolicy in kyverno - in that case, I need to know the cluster has "kyverno" capability with a sufficiently new version - so capabilities also need versions, I think.
One important note: I think that if the "capabilities" are not specific enough for you, you can just ignore them and fetch the real list of CRDs using just the k8s API. This will give you all the details possible and won't be harder than getting the same list from a toolkit while at the same time not imposing a new feature on the contract.
There was a problem hiding this comment.
My point is only the test probably depends on something specific, and people are not going to use these how you expect, so it'll become more headache for you to manage.
Versioning is a good idea in any case if you choose to do capabilities
|
|
||
| The proposed config schema is (including the default values and respective env vars for overrides): | ||
|
|
||
| ```yaml |
There was a problem hiding this comment.
I suggest versioning the schema
The goal here is "write tests once, run in both". There will be no more "atf tests" and "ats tests", as both toolkits will be able to execute them (after applying the contract). As for making this a single tool, that runs different clusters depending on the requested environment: totally possible, but the idea here is about saving us a major rewrite, and saving us time. |
|
As a general comment: thanks for all the detailed comments, they will become very useful if we accept the overall idea. But the core question right now is "do we find this ok, as it is a compromise of some sort"? |
stone-z
left a comment
stone-z
left a comment
There was a problem hiding this comment.
The goal here is "write tests once, run in both". There will be no more "atf tests" and "ats tests", as both toolkits will be able to execute them (after applying the contract).
Ok I think I get it now. So, ats will be updated to also run Go tests, and atf will be updated to also run Python tests.
But only ats will be able to run in kind / CircleCI.
So HB and Tenet still need to maintain, and now keep feature parity in, two tools in parallel.
If ats can run everywhere, and can now run Go tests, what is the upside of duplicating that in atf?
As for making this a single tool, that runs different clusters depending on the requested environment: totally possible, but the idea here is about saving us a major rewrite, and saving us time.
Is updating two tools with new language support inherently any faster than updating one tool (atf) to call another existing tool (ats).
atf and ats still seem like different things to me, but I think it's ultimately up to the two teams if they would prefer to maintain parallel tools.
The scope of the RFC is still unclear based on comments. If this is going to be the definitive contract decision, then I would like to see more specific consideration for nested charts and cluster lifecycle tests, and I'd have more input there. If you want to keep this a high-level "we want HB and Tenet to make our tools do the same things, and we will get back to you with a more specific RFC about what you need from the contract" then I'm perfectly happy if you drop the specific API and implementation discussion from this RFC and continue with that plan.
| type. | ||
| 1. `post-run` hook runs for `type` tests (if present). | ||
| 1. If there are `upgrade` tests: | ||
| 1. If the version under test (new) is already installed in the cluster, it is uninstalled. |
There was a problem hiding this comment.
Well, you're including the hooks and test flow as part of the RFC. So if it's something to be addressed, I think it should be, or if the test sequencing is something that is planned to be flexible and managed separately, then maybe it should be removed?
| type. | ||
| 1. `post-run` hook runs for `type` tests (if present). | ||
| 1. If there are `upgrade` tests: | ||
| 1. If the version under test (new) is already installed in the cluster, it is uninstalled. |
There was a problem hiding this comment.
Same comment here - the RFC describes the hooks that are possible, so since cluster lifecycle tests are relevant, it would be good to commit to some support for them in the RFC.
Or, if the RFC is intended to still be higher-level, then the implementation details don't belong here.
| will probably be able to deliver some of these capabilities. To cope with these differences, tests that need | ||
| those capabilities should gate on the `APP_TEST_CAPABILITIES`. | ||
|
|
||
| We define the following capabilities, which a toolkit might set in `APP_TEST_CAPABILITIES`: |
There was a problem hiding this comment.
My point is only the test probably depends on something specific, and people are not going to use these how you expect, so it'll become more headache for you to manage.
Versioning is a good idea in any case if you choose to do capabilities
| Tests live in one directory: `tests/app/`. Any harness that deploys the app runs that directory the same way. | ||
| An empty `tests/app/` (no module, no project) isn't an opt-in and is skipped. | ||
|
|
||
| It's either one Go module or one Python project, not both. The runner picks the executor from what's there: |
There was a problem hiding this comment.
I was just clarifying. I don't necessarily want to have both. I'd prefer to write tests in Go, but it's more important to me that I can run tests everywhere, so if that means Python I'll do that.
Not supporting both in parallel just reinforces my top-level comment.
|
Stepping in as a stakeholder rather than a reviewer, because I think this thread has two arguments tangled into one and that's why it isn't converging. Argument 1: how do we author app tests? One directory, three test types, env-var inputs, JUnit output, tests never provision. Reading back through the thread, nobody has actually objected to any of this. Mati and Zach are raising valid detail questions (capabilities vs. CRD probing, cluster-lifecycle upgrades, schema versioning) but not disputing the authoring model itself. Argument 2: how many runners execute it? This is the real disagreement, and it's an architectural call about The important thing is that these are independent. The contract does not commit us to keeping two runners forever. If anything it's what makes collapsing to one runner affordable later: tests written against the contract survive that migration unchanged, whereas tests written against a harness API do not. Mati and Zach, if your position is that we should end up on one tool, the contract is on your side — it decouples the test corpus from the decision, so we can make it on its own merits and on its own timeline instead of holding app testing hostage to it. So my proposal:
One thing worth saying out loud: we have two overlapping test tools, owned by two teams, with close to zero adoption of either. That's the underlying problem. A contract is a reasonable way to stop the bleeding while we decide, but it isn't the fix, and I don't want merging it to feel like the question is closed. |
This is not 100% true, because the current model doesn't cover all the With that, I see that we (cabbage) will never migrate to the proposed contract and rely on skipping the ATS validation at all, just to avoid maintaining 2 set of tests. What this is trying to solve is causing the completely contrary effect. |
|
from sig arch - @mcharriere plz share more thoughts <3 |
7 participants
Defines a harness-neutral contract so the same app test files run under both app-test-suite (fast checks per PR on kind) and apptest-framework (full e2e nightly on workload clusters).
We are keeping both harnesses on purpose: they are a deliberate fast/slow split, not redundant tools. The motivation is not deduping existing tests (there is little to dedup); it is that the two harnesses had divergent authoring models, so covering both cadences cost 2x from scratch and adoption of both was close to zero. This contract unifies the authoring model: one directory, one idiom, both runners.
Core of the proposal:
tests/app/), one Go module or one Python project, presence is the opt-in; committed lockfiles, installed frozen/offlinesmoke,functional,upgrade, matching ATS's existing StepTypes); the upgrade flow runs onlyupgrade-typed tests, before and after the upgrade (APP_TEST_UPGRADE_STAGE), never re-running smoke/functional; the pre run is the baseline that attributes a post failure to the upgrade; pure side-effect seeding goes in apre-upgradehookKUBECONFIG+ canonicalAPP_TEST_*env vars (legacyATS_*exported alongside); tests never provision/runfull-flow trigger to pull the signal forwardupgrade-typed tests (no config switch); optionalexpectedTypesmakes named types mandatory, so a typo'd tag fails instead of silently skippingGrew out of the discussion around unifying chart and e2e test authoring (pilot groundwork: giantswarm/muster#954). Stakeholders: team-honeybadger (app-test-suite), team-tenet (apptest-framework), team-bumblebee.