Skip to content

Add DAFFODIL_TDML_TUNABLES to run TDML suites under tunables - #1745

Open
olabusayoT wants to merge 2 commits into
apache:mainfrom
olabusayoT:daf-3065-tdml-env-tunables
Open

olabusayoT wants to merge 2 commits into
apache:mainfrom
olabusayoT:daf-3065-tdml-env-tunables

Conversation

@olabusayoT

Copy link
Copy Markdown
Contributor

The DAFFODIL_TDML_TUNABLES environment variable holds a comma-separated list of name=value tunables that apply to every TDML test. A test's own defineConfig tunables override them, so a whole suite can run under any combination of tunables without editing any schema or TDML file.

The list is parsed by a small helper that ignores blank entries, trims whitespace, lets a value contain '=', and lets a later duplicate win. An entry with no name or no '=' fails with an error that names the entry, so a typo in the variable is reported instead of silently ignored. A tunable name Daffodil does not know is rejected when the test compiles, as it would be from defineConfig.

Unit tests cover each parsing case.

(Pulled from the prefetch PR to simplify review #1736; also #1743 contains a Misc function that this will benefit from so should be updated to use it once that PR is merged)

DAFFODIL-3065

The DAFFODIL_TDML_TUNABLES environment variable holds a comma-separated list of name=value tunables that apply to every TDML test. A test's own defineConfig tunables override them, so a whole suite can run under any combination of tunables without editing any schema or TDML file.

The list is parsed by a small helper that ignores blank entries, trims whitespace, lets a value contain '=', and lets a later duplicate win. An entry with no name or no '=' fails with an error that names the entry, so a typo in the variable is reported instead of silently ignored. A tunable name Daffodil does not know is rejected when the test compiles, as it would be from defineConfig.

Unit tests cover each parsing case.

DAFFODIL-3065

@stevedlawrence stevedlawrence left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, just suggest alternate tests

TDMLEnvTunables.parse("=1")
}
assertTrue(e.getMessage.contains("expected name=value"))
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, these are just testing some fairly basic parsing of strings and creating a map, which we do all over Daffodil without explicitly unit tests. And more important, these tests miss on making sure the tunables are actually used in a tdml test.

We should be careful that as we start adding more capabilities that we make sure AI isn't adding tests that don't add much value. A few here and there don't matter, but as AI is used more and more we could get to a point where it starts causing testing to take longer without much real confidence in the code or value gained.

What about replacing these with a functional test that makes sure the env variable works as expected? Maybe we add a CLI tdml test that sets the environment variable and expects a test to fail because of the value, confirming that the value was parsed correctly and used? For example, maybe you set the maxHexBinaryLengthInBytes tunable to 1 and show that a test fails to parse the hex binary, and then set it to 100 and show that a test passes. You could have multiple variables with a few different tests to make sure certain tests pass/fail as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants