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

Add an acc test covering failures when reading .git #2223

Merged
merged 5 commits into from
Jan 24, 2025

Conversation

denik
Copy link
Contributor

@denik denik commented Jan 24, 2025

Changes

  • New test covering failures in reading .git. One case results in error, some result in warning (not shown).
  • New helper withdir runs commands in a subdirectory.

Tests

New acceptance test.

local exit_code=$?
cd "$orig_dir" || return $?
return $exit_code
}
Copy link
Contributor

Choose a reason for hiding this comment

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

It's easier/shorter to just use a subshell.

E.g.

(
  cd subdir/a/b
  errcode trace $CLI bundle validate -o json | jq .bundle.git
)

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 mean using subshell directly in script?

It's not shorter, especially if you consider that you also want to trace cd command and then you have it appearing in the output as a separate line.

Copy link
Contributor Author

@denik denik Jan 24, 2025

Choose a reason for hiding this comment

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

So we're comparing:

script:
errcode trace withdir subdir/a/b $CLI bundle validate -o json | jq .bundle.git

output:
>>> withdir subdir/a/b $CLI bundle validate -o json

vs

script:

(
    trace cd subdir/a/b
    errcode trace $CLI bundle validate -o json | jq .bundle.git
)

output:

>>> cd subdir/a/b
>>> $CLI bundle validate -o json

Note, in the output you cannot see that you're inside a subshell and that cd subdir/a/b only applies to next statement.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense, thanks.

@denik denik temporarily deployed to test-trigger-is January 24, 2025 10:08 — with GitHub Actions Inactive
@denik denik force-pushed the DECO-24321--denik/git-permerror branch from 311d3f9 to 15b5b8a Compare January 24, 2025 11:34
@denik denik temporarily deployed to test-trigger-is January 24, 2025 11:34 — with GitHub Actions Inactive
@denik denik temporarily deployed to test-trigger-is January 24, 2025 11:39 — with GitHub Actions Inactive
@denik denik force-pushed the DECO-24321--denik/git-permerror branch from 65a6507 to b58a5e8 Compare January 24, 2025 11:41
@denik denik temporarily deployed to test-trigger-is January 24, 2025 11:41 — with GitHub Actions Inactive
@denik denik force-pushed the DECO-24321--denik/git-permerror branch from b58a5e8 to ccd1f1f Compare January 24, 2025 13:53
@denik denik temporarily deployed to test-trigger-is January 24, 2025 13:53 — with GitHub Actions Inactive
@denik denik temporarily deployed to test-trigger-is January 24, 2025 13:55 — with GitHub Actions Inactive
github-merge-queue bot pushed a commit that referenced this pull request Jan 24, 2025
## Changes
- Acceptance tests load test.toml to configure test behaviour.
- If file is not found in the test directory, parents are searched,
until the test root.
- Currently there is one option: runtime.GOOS to switch off tests per
OS.

## Tests
Using it in #2223 to disable test
on Windows that cannot be run there.
@denik denik force-pushed the DECO-24321--denik/git-permerror branch from fb41bad to e3c5225 Compare January 24, 2025 15:29
@denik denik temporarily deployed to test-trigger-is January 24, 2025 15:29 — with GitHub Actions Inactive
@denik denik enabled auto-merge January 24, 2025 15:30
local exit_code=$?
cd "$orig_dir" || return $?
return $exit_code
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense, thanks.

git-repo-init
mkdir -p subdir/a/b

printf "=== No permission to access .git. Badness: inferred flag is set to true even though we did not infer branch. bundle_root_path is not correct in subdir case.\n"
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 use the "title" helper instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tbh, that helper feels unnecessary to me. It also does not add a new line at the end for some reason.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, in some cases I want a couple of front newlines and in one case I won't none.

@denik denik added this pull request to the merge queue Jan 24, 2025
Merged via the queue into main with commit 468660d Jan 24, 2025
9 checks passed
@denik denik deleted the DECO-24321--denik/git-permerror branch January 24, 2025 15:58
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