Skip to content

[Dev] Fix "environment variable already defined" error in sqllogictest when test_env is used - #21305

Merged
Mytherin merged 3 commits into
duckdb:v1.5-variegatafrom
Tishj:test_env_require_env_fixes
Jun 11, 2026
Merged

Mytherin merged 3 commits into
duckdb:v1.5-variegatafrom
Tishj:test_env_require_env_fixes

Conversation

@Tishj

@Tishj Tishj commented Mar 11, 2026

Copy link
Copy Markdown
Member

This PR is a follow-up to #21099

Problem is fixed by making sure test_env gets loaded only once, and we make sure environment variables provided by test_env don't get falsely flagged as "double defined"

@Tishj

Tishj commented Mar 11, 2026

Copy link
Copy Markdown
Member Author

I can see this:

	// setup this test runner with Config-based env, then override with ephemerals (only WORKING_DIR at this point)
	for (auto &kv : test_config.GetTestEnvMap()) {
		runner.environment_variables[kv.first] = kv.second;
	}

And the environment_variables are part of the runner, so yes they are shared between all files.
That doesn't explain why the same require-env in different tests doesn't cause this same error to be thrown.

Aha, figured it out
GetTestEnvMap returns test_env
test_env is lazily populated by TestConfiguration::GetTestEnv, which we now use in resolving require-env

So for the next run, test_env will contain the environment variables that were listed by all previous require-env items, and will populate runner.environment_variables with those key-value pairs

@github-actions

Copy link
Copy Markdown

This pull request is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions Bot added the stale label Apr 11, 2026
@github-actions

Copy link
Copy Markdown

This pull request was closed because it has been stale for 30 days with no activity.

@github-actions github-actions Bot closed this May 11, 2026
@Tishj Tishj reopened this Jun 10, 2026
@github-actions github-actions Bot removed the stale label Jun 11, 2026
@Tishj
Tishj requested a review from pdet June 11, 2026 06:31
@Mytherin
Mytherin merged commit dc6a0be into duckdb:v1.5-variegata Jun 11, 2026
59 checks passed
@Mytherin

Copy link
Copy Markdown
Collaborator

Thanks!

krlmlr pushed a commit to krlmlr/duckdb that referenced this pull request Jun 21, 2026
Conflict-free merge of the clean commits between resolutions:
  - Fix crash when storage path is not set (duckdb#23174)
  - Zero the inlined buffer in string_t's length-only constructor (duckdb#23201)
  - [Dev] Fix "environment variable already defined" error in sqllogictest when `test_env` is used (duckdb#21305)
  - parquet: initialize `ParquetReader::rows_read` (duckdb#23205)
krlmlr pushed a commit to krlmlr/duckdb that referenced this pull request Jun 21, 2026
…t when `test_env` is used (duckdb#21305)

This PR is a follow-up to duckdb#21099 

Problem is fixed by making sure `test_env` gets loaded only once, and we
make sure environment variables provided by `test_env` don't get falsely
flagged as "double defined"
krlmlr pushed a commit to krlmlr/duckdb that referenced this pull request Jun 21, 2026
Conflict-free merge of the clean commits between resolutions:
  - Fix crash when storage path is not set (duckdb#23174)
  - Zero the inlined buffer in string_t's length-only constructor (duckdb#23201)
  - [Dev] Fix "environment variable already defined" error in sqllogictest when `test_env` is used (duckdb#21305)
  - parquet: initialize `ParquetReader::rows_read` (duckdb#23205)
krlmlr pushed a commit to krlmlr/duckdb that referenced this pull request Jun 21, 2026
Brings these v1.5-variegata commits; they do not overlap with the main/v2.0
line, so they merge with no conflicts:

- Fix crash when storage path is not set (duckdb#23174)
- Zero the inlined buffer in string_t's length-only constructor (duckdb#23201)
- [Dev] Fix "environment variable already defined" error in sqllogictest when `test_env` is used (duckdb#21305)
- parquet: initialize `ParquetReader::rows_read` (duckdb#23205)
krlmlr pushed a commit to krlmlr/duckdb that referenced this pull request Jun 21, 2026
Brings these v1.5-variegata commits; they do not overlap with the main/v2.0
line, so they merge with no conflicts:

- Fix crash when storage path is not set (duckdb#23174)
- Zero the inlined buffer in string_t's length-only constructor (duckdb#23201)
- [Dev] Fix "environment variable already defined" error in sqllogictest when `test_env` is used (duckdb#21305)
- parquet: initialize `ParquetReader::rows_read` (duckdb#23205)
kaka11chen pushed a commit to kaka11chen/astrovela_vane that referenced this pull request Aug 14, 2026
…t when `test_env` is used (duckdb/duckdb#21305)

This PR is a follow-up to #21099 

Problem is fixed by making sure `test_env` gets loaded only once, and we
make sure environment variables provided by `test_env` don't get falsely
flagged as "double defined"

Upstream-Commit: duckdb/duckdb@dc6a0be
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