Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Env var run summary data generation #4529

Merged
merged 7 commits into from Apr 18, 2023
+450 −73
Merged

Env var run summary data generation #4529

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
94e7806
Add data to run summary.
nathanhammond Apr 11, 2023
07aab05
Marshal to lowercase.
nathanhammond Apr 12, 2023
ea6592c
Fix issues
nathanhammond Apr 12, 2023
bc86c30
Address feedback.
nathanhammond Apr 13, 2023
bb04a01
make linter happy
nathanhammond Apr 13, 2023
5ad4d22
Additional tests for run summary outputs.
nathanhammond Apr 13, 2023
c4f701d
Combine env var config fixtures.
nathanhammond Apr 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
0 .../strict_env_vars/fixture-configs/all.json → ...fixtures/strict_env_vars_configs/all.json
View file
Open in desktop
File renamed without changes.
0 ...vars/fixture-configs/global_pt-empty.json → ...ict_env_vars_configs/global_pt-empty.json
View file
Open in desktop
File renamed without changes.
0 ...t_env_vars/fixture-configs/global_pt.json → ...es/strict_env_vars_configs/global_pt.json
View file
Open in desktop
File renamed without changes.
0 ...v_vars/fixture-configs/task_pt-empty.json → ...trict_env_vars_configs/task_pt-empty.json
View file
Open in desktop
File renamed without changes.
0 ...ict_env_vars/fixture-configs/task_pt.json → ...ures/strict_env_vars_configs/task_pt.json
View file
Open in desktop
File renamed without changes.
15 changes: 12 additions & 3 deletions cli/integration_tests/dry_json/monorepo.t
View file
Open in desktop
Expand Up @@ -72,6 +72,7 @@ Setup

$ cat tmpjson.log | jq 'keys'
[
"envMode",
"globalCacheInputs",
"id",
"packages",
Expand Down Expand Up @@ -122,13 +123,16 @@ Setup
},
"expandedOutputs": [],
"framework": "<NO FRAMEWORK DETECTED>",
"envMode": "loose",
"environmentVariables": {
"configured": [],
"inferred": [],
"global": [
"SOME_ENV_VAR=",
"VERCEL_ANALYTICS_ID="
]
],
"passthrough": null,
"globalPassthrough": null
nathanhammond marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down Expand Up @@ -170,6 +174,7 @@ Setup
},
"expandedOutputs": [],
"framework": "<NO FRAMEWORK DETECTED>",
"envMode": "loose",
"environmentVariables": {
"configured": [
"NODE_ENV="
Expand All @@ -178,7 +183,9 @@ Setup
"global": [
"SOME_ENV_VAR=",
"VERCEL_ANALYTICS_ID="
]
],
"passthrough": null,
"globalPassthrough": null
}
}

Expand All @@ -192,7 +199,9 @@ Run again with NODE_ENV set and see the value in the summary. --filter=util work
"global": [
"SOME_ENV_VAR=",
"VERCEL_ANALYTICS_ID="
]
],
"passthrough": null,
"globalPassthrough": null
}

Tasks that don't exist throw an error
Expand Down
6 changes: 5 additions & 1 deletion cli/integration_tests/dry_json/single_package.t
View file
Open in desktop
Expand Up @@ -29,6 +29,7 @@ Setup
}
}
},
"envMode": "infer",
"tasks": [
{
"taskId": "build",
Expand Down Expand Up @@ -70,12 +71,15 @@ Setup
},
"expandedOutputs": [],
"framework": "\u003cNO FRAMEWORK DETECTED\u003e",
"envMode": "loose",
"environmentVariables": {
"configured": [],
"inferred": [],
"global": [
"VERCEL_ANALYTICS_ID="
]
],
"passthrough": null,
"globalPassthrough": null
}
}
]
Expand Down
6 changes: 5 additions & 1 deletion cli/integration_tests/dry_json/single_package_no_config.t
View file
Open in desktop
Expand Up @@ -26,6 +26,7 @@ Setup
}
}
},
"envMode": "infer",
"tasks": [
{
"taskId": "build",
Expand Down Expand Up @@ -61,12 +62,15 @@ Setup
},
"expandedOutputs": [],
"framework": "\u003cNO FRAMEWORK DETECTED\u003e",
"envMode": "loose",
"environmentVariables": {
"configured": [],
"inferred": [],
"global": [
"VERCEL_ANALYTICS_ID="
]
],
"passthrough": null,
"globalPassthrough": null
}
}
]
Expand Down
11 changes: 9 additions & 2 deletions cli/integration_tests/dry_json/single_package_with_deps.t
View file
Open in desktop
Expand Up @@ -39,6 +39,7 @@ Setup
}
}
},
"envMode": "infer",
"tasks": [
{
"taskId": "build",
Expand Down Expand Up @@ -81,12 +82,15 @@ Setup
},
"expandedOutputs": [],
"framework": "\u003cNO FRAMEWORK DETECTED\u003e",
"envMode": "loose",
"environmentVariables": {
"configured": [],
"inferred": [],
"global": [
"VERCEL_ANALYTICS_ID="
]
],
"passthrough": null,
"globalPassthrough": null
}
},
{
Expand Down Expand Up @@ -128,12 +132,15 @@ Setup
},
"expandedOutputs": [],
"framework": "\u003cNO FRAMEWORK DETECTED\u003e",
"envMode": "loose",
"environmentVariables": {
"configured": [],
"inferred": [],
"global": [
"VERCEL_ANALYTICS_ID="
]
],
"passthrough": null,
"globalPassthrough": null
}
}
]
Expand Down
5 changes: 4 additions & 1 deletion cli/integration_tests/run_summary/error.t
View file
Open in desktop
Expand Up @@ -63,13 +63,16 @@ Validate that we got a full task summary for the failed task with an error in .e
},
"expandedOutputs": [],
"framework": "",
"envMode": "loose",
"environmentVariables": {
"configured": [],
"inferred": [],
"global": [
"SOME_ENV_VAR=",
"VERCEL_ANALYTICS_ID="
]
],
"passthrough": null,
"globalPassthrough": null
},
"execution": {
"startTime": [0-9]+, (re)
Expand Down
1 change: 1 addition & 0 deletions cli/integration_tests/run_summary/single-package.t
View file
Open in desktop
Expand Up @@ -51,6 +51,7 @@ Check
"command",
"dependencies",
"dependents",
"envMode",
"environmentVariables",
"excludedOutputs",
"execution",
Expand Down
206 changes: 206 additions & 0 deletions cli/integration_tests/run_summary/strict_env.t
View file
Open in desktop
@@ -0,0 +1,206 @@
Setup
$ . ${TESTDIR}/../_helpers/setup.sh
$ . ${TESTDIR}/../_helpers/setup_monorepo.sh $(pwd) strict_env_vars

Set the env vars
$ export GLOBAL_VAR_PT=higlobalpt
$ export GLOBAL_VAR_DEP=higlobaldep
$ export LOCAL_VAR_PT=hilocalpt
$ export LOCAL_VAR_DEP=hilocaldep
$ export OTHER_VAR=hiother
$ export SYSTEMROOT=hisysroot

Run as `infer`
$ rm -rf .turbo/runs
$ ${TURBO} run build --summarize > /dev/null
$ cat .turbo/runs/*.json | jq -r '.envMode'
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand that it's unnecessary if you're rm -rf'ing right before., but all the other tests explicitly get the first one with head -n1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mentioned that this didn't guarantee ordering in some other test so I took this strategy.

Why was the ksuid ordering not guaranteed?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't remember the details to dig it up again, but I'm 99% sure that was an incorrect statement and I solved the issue I was seeing a different way. I think it's still safe to rely on ksuid ordering. (That said, it's not a requirement since rm && cat already does what we need).

infer
$ cat .turbo/runs/*.json | jq -r '.tasks[0].envMode'
loose
$ cat .turbo/runs/*.json | jq -r '.tasks[0].environmentVariables | {passthrough, globalPassthrough}'
{
"passthrough": null,
"globalPassthrough": null
}

Run as `strict`
$ rm -rf .turbo/runs
$ ${TURBO} run build --experimental-env-mode=strict --summarize > /dev/null
$ cat .turbo/runs/*.json | jq -r '.envMode'
strict
$ cat .turbo/runs/*.json | jq -r '.tasks[0].envMode'
strict
$ cat .turbo/runs/*.json | jq -r '.tasks[0].environmentVariables | {passthrough, globalPassthrough}'
{
"passthrough": null,
"globalPassthrough": null
}

Run as `loose`
$ rm -rf .turbo/runs
$ ${TURBO} run build --experimental-env-mode=loose --summarize > /dev/null
$ cat .turbo/runs/*.json | jq -r '.envMode'
loose
$ cat .turbo/runs/*.json | jq -r '.tasks[0].envMode'
loose
$ cat .turbo/runs/*.json | jq -r '.tasks[0].environmentVariables | {passthrough, globalPassthrough}'
{
"passthrough": null,
"globalPassthrough": null
}

All specified + infer
$ rm -rf .turbo/runs
$ cp "$TESTDIR/../_fixtures/strict_env_vars_configs/all.json" "$(pwd)/turbo.json" && git commit --allow-empty -am "no comment" --quiet
$ ${TURBO} run build --summarize > /dev/null
$ cat .turbo/runs/*.json | jq -r '.envMode'
strict
$ cat .turbo/runs/*.json | jq -r '.tasks[0].envMode'
strict
$ cat .turbo/runs/*.json | jq -r '.tasks[0].environmentVariables | {passthrough, globalPassthrough}'
{
"passthrough": [
"LOCAL_VAR_PT=7cd1bb19c058cf4d6ad6aa579d685bddddf3ab587b78bdcb1e6e488fb6f47a3b"
],
"globalPassthrough": [
"GLOBAL_VAR_PT=cecd31fff1e723588eac8fe1edff89a6d2ec072f5c3bd884a98297487670b1f0"
]
}

All specified + loose
$ rm -rf .turbo/runs
$ cp "$TESTDIR/../_fixtures/strict_env_vars_configs/all.json" "$(pwd)/turbo.json" && git commit --allow-empty -am "no comment" --quiet
$ ${TURBO} run build --experimental-env-mode=loose --summarize > /dev/null
$ cat .turbo/runs/*.json | jq -r '.envMode'
loose
$ cat .turbo/runs/*.json | jq -r '.tasks[0].envMode'
loose
$ cat .turbo/runs/*.json | jq -r '.tasks[0].environmentVariables | {passthrough, globalPassthrough}'
{
"passthrough": [
"LOCAL_VAR_PT=7cd1bb19c058cf4d6ad6aa579d685bddddf3ab587b78bdcb1e6e488fb6f47a3b"
],
"globalPassthrough": [
"GLOBAL_VAR_PT=cecd31fff1e723588eac8fe1edff89a6d2ec072f5c3bd884a98297487670b1f0"
]
}

Global passthrough specified empty array + infer
$ rm -rf .turbo/runs
$ cp "$TESTDIR/../_fixtures/strict_env_vars_configs/global_pt-empty.json" "$(pwd)/turbo.json" && git commit --allow-empty -am "no comment" --quiet
$ ${TURBO} run build --summarize > /dev/null
$ cat .turbo/runs/*.json | jq -r '.envMode'
strict
$ cat .turbo/runs/*.json | jq -r '.tasks[0].envMode'
strict
$ cat .turbo/runs/*.json | jq -r '.tasks[0].environmentVariables | {passthrough, globalPassthrough}'
{
"passthrough": null,
"globalPassthrough": []
}

Global passthrough specified value + infer
$ rm -rf .turbo/runs
$ cp "$TESTDIR/../_fixtures/strict_env_vars_configs/global_pt.json" "$(pwd)/turbo.json" && git commit --allow-empty -am "no comment" --quiet
$ ${TURBO} run build --summarize > /dev/null
$ cat .turbo/runs/*.json | jq -r '.envMode'
strict
$ cat .turbo/runs/*.json | jq -r '.tasks[0].envMode'
strict
$ cat .turbo/runs/*.json | jq -r '.tasks[0].environmentVariables | {passthrough, globalPassthrough}'
{
"passthrough": null,
"globalPassthrough": [
"GLOBAL_VAR_PT=cecd31fff1e723588eac8fe1edff89a6d2ec072f5c3bd884a98297487670b1f0"
]
}

Global passthrough specified empty array + loose
$ rm -rf .turbo/runs
$ cp "$TESTDIR/../_fixtures/strict_env_vars_configs/global_pt-empty.json" "$(pwd)/turbo.json" && git commit --allow-empty -am "no comment" --quiet
$ ${TURBO} run build --experimental-env-mode=loose --summarize > /dev/null
$ cat .turbo/runs/*.json | jq -r '.envMode'
loose
$ cat .turbo/runs/*.json | jq -r '.tasks[0].envMode'
loose
$ cat .turbo/runs/*.json | jq -r '.tasks[0].environmentVariables | {passthrough, globalPassthrough}'
{
"passthrough": null,
"globalPassthrough": []
}

Global passthrough specified value + loose
$ rm -rf .turbo/runs
$ cp "$TESTDIR/../_fixtures/strict_env_vars_configs/global_pt.json" "$(pwd)/turbo.json" && git commit --allow-empty -am "no comment" --quiet
$ ${TURBO} run build --experimental-env-mode=loose --summarize > /dev/null
$ cat .turbo/runs/*.json | jq -r '.envMode'
loose
$ cat .turbo/runs/*.json | jq -r '.tasks[0].envMode'
loose
$ cat .turbo/runs/*.json | jq -r '.tasks[0].environmentVariables | {passthrough, globalPassthrough}'
{
"passthrough": null,
"globalPassthrough": [
"GLOBAL_VAR_PT=cecd31fff1e723588eac8fe1edff89a6d2ec072f5c3bd884a98297487670b1f0"
]
Copy link
Contributor

Choose a reason for hiding this comment

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

Fine since we're still experimental, but I was under the impression that the value of pass through vars is not part of the hash. If that's still the case, the run summary could also probably omit the values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that your concern is unnecessarily conflating "is part of run summary" with "is part of hash". There are lots of things in the output that aren't (intentionally) included in the hash so this doesn't strike me as a problem. We can address this in any UI: "highlight the hash inputs".

I explicitly went through to add this because being able to unmask that value in the future if you can guess the input to the hash is super-valuable to discover why the run outputs are what they are.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that your concern is unnecessarily conflating "is part of run summary" with "is part of hash"

I didn't intend it as that. My view is that the summary is a good way to understand what the tool did. If it's not doing anything with a thing, it doesn't need to be reflected back to the user. In this case, my earlier comment is promoted by the fact that we do something with the env var keys, but nothing with their values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But we are doing something with it! We're passing that KV env pair into the environment! And it's going to be different on different people's boxes, but the hash will still be the same!

Of all the places where we decide whether or not to capture this information I believe this is the most important one to capture.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure we're doing something with it. But said a different way, turbo doesn't care about the values of those env vars. Turbo is concerned with the list of keys. For globalEnv, Turbo cares about both the keys and the values. That's the difference I'm trying to point to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's really my distinction too: turbo may not care other than shuffling the correct things into the environment, but the user definitely cares what actually ended up there.

We should be doing things for the user.

}

Task passthrough specified empty array + infer
$ rm -rf .turbo/runs
$ cp "$TESTDIR/../_fixtures/strict_env_vars_configs/task_pt-empty.json" "$(pwd)/turbo.json" && git commit --allow-empty -am "no comment" --quiet
$ ${TURBO} run build --summarize > /dev/null
$ cat .turbo/runs/*.json | jq -r '.envMode'
infer
$ cat .turbo/runs/*.json | jq -r '.tasks[0].envMode'
strict
$ cat .turbo/runs/*.json | jq -r '.tasks[0].environmentVariables | {passthrough, globalPassthrough}'
{
"passthrough": [],
"globalPassthrough": null
}

Task passthrough specified value + infer
$ rm -rf .turbo/runs
$ cp "$TESTDIR/../_fixtures/strict_env_vars_configs/task_pt.json" "$(pwd)/turbo.json" && git commit --allow-empty -am "no comment" --quiet
$ ${TURBO} run build --summarize > /dev/null
$ cat .turbo/runs/*.json | jq -r '.envMode'
infer
$ cat .turbo/runs/*.json | jq -r '.tasks[0].envMode'
strict
$ cat .turbo/runs/*.json | jq -r '.tasks[0].environmentVariables | {passthrough, globalPassthrough}'
{
"passthrough": [
"LOCAL_VAR_PT=7cd1bb19c058cf4d6ad6aa579d685bddddf3ab587b78bdcb1e6e488fb6f47a3b"
],
"globalPassthrough": null
}

Task passthrough specified empty array + loose
$ rm -rf .turbo/runs
$ cp "$TESTDIR/../_fixtures/strict_env_vars_configs/task_pt-empty.json" "$(pwd)/turbo.json" && git commit --allow-empty -am "no comment" --quiet
$ ${TURBO} run build --experimental-env-mode=loose --summarize > /dev/null
$ cat .turbo/runs/*.json | jq -r '.envMode'
loose
$ cat .turbo/runs/*.json | jq -r '.tasks[0].envMode'
loose
$ cat .turbo/runs/*.json | jq -r '.tasks[0].environmentVariables | {passthrough, globalPassthrough}'
{
"passthrough": [],
"globalPassthrough": null
}

Task passthrough specified value + loose
$ rm -rf .turbo/runs
$ cp "$TESTDIR/../_fixtures/strict_env_vars_configs/task_pt.json" "$(pwd)/turbo.json" && git commit --allow-empty -am "no comment" --quiet
$ ${TURBO} run build --experimental-env-mode=loose --summarize > /dev/null
$ cat .turbo/runs/*.json | jq -r '.envMode'
loose
$ cat .turbo/runs/*.json | jq -r '.tasks[0].envMode'
loose
$ cat .turbo/runs/*.json | jq -r '.tasks[0].environmentVariables | {passthrough, globalPassthrough}'
{
"passthrough": [
"LOCAL_VAR_PT=7cd1bb19c058cf4d6ad6aa579d685bddddf3ab587b78bdcb1e6e488fb6f47a3b"
],
"globalPassthrough": null
}