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.

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

Restore variable file tests #2220

Merged
merged 8 commits into from
Jan 29, 2025
Merged

Restore variable file tests #2220

merged 8 commits into from
Jan 29, 2025

Conversation

ilyakuz-db
Copy link
Contributor

Changes

Uncomment flaky tests, they work properly with latest changes from main

Tests

@denik
Copy link
Contributor

denik commented Jan 23, 2025

Windows failed due to slashes, can you try adding | sed 's/\\/\//g'.

# title "file cannot be parsed"
# trace errcode $CLI bundle validate -o json --target invalid_json | jq $cluster_expr
title "file cannot be parsed"
trace errcode $CLI bundle validate -o json --target invalid_json | jq $cluster_expr | sed 's/\\/\//g'
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be fixed with:

trace errcode $CLI bundle validate -o json --target invalid_json 2>&1 | sed 's/\\/\//g'

The jq expression is not needed because the command fails. The error is written to stderr so we need a redirect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The jq expression is not needed because the command fails

Otherwise it will print all json output in addition to the error, example

Copy link
Contributor

@denik denik Jan 28, 2025

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you use Repls instead of sed? See example: https://github.com/databricks/cli/blob/main/acceptance/selftest/test.toml#L17

Looks good, let me try

# Fix for windows
[[Repls]]
Old = '\$TMPDIR\\.databricks\\bundle\\wrong_file_structure\\variable-overrides.json'
New = '$TMPDIR/.databricks/bundle/wrong_file_structure/variable-overrides.json'
Copy link
Contributor

Choose a reason for hiding this comment

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

The $ is interpreted as a reference to match. You can indicate a $ literal with $$:

https://pkg.go.dev/regexp#Regexp.Expand

@ilyakuz-db ilyakuz-db added this pull request to the merge queue Jan 29, 2025
Merged via the queue into main with commit 59d6fbf Jan 29, 2025
9 checks passed
@ilyakuz-db ilyakuz-db deleted the more-var-file-tests branch January 29, 2025 13:40
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.

3 participants