Skip to content

Add a test for sync root outside of git root#2202

Merged
denik merged 5 commits intomainfrom
denik/sync-root-test
Jan 22, 2025
Merged

Add a test for sync root outside of git root#2202
denik merged 5 commits intomainfrom
denik/sync-root-test

Conversation

@denik
Copy link
Copy Markdown
Contributor

@denik denik commented Jan 21, 2025

  • Move acceptance/bundle/sync-paths-dotdot test to acceptance/bundle/syncroot/dotdot-notgit
  • Add new test acceptance/bundle/syncroot/dotdot-git

Fix replacer to work with this test and on Windows:

  • Make PATH work on Windows by using EvalSymlinks.
  • Make concatenated path match within JSON but stripping quotes.

@@ -0,0 +1,6 @@
# This should error, we do not allow syncroot outside of git repo
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there a reason why we won't allow it? I can imagine the case when users want to sync build artifacts which are not part of the git repo as well

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The idea here is that if people use git, they want reproducibility, so sources should be in git.

build artifacts can be placed inside git root and gitignored.

@denik denik temporarily deployed to test-trigger-is January 21, 2025 16:20 — with GitHub Actions Inactive
@denik denik temporarily deployed to test-trigger-is January 22, 2025 09:41 — with GitHub Actions Inactive
@denik denik temporarily deployed to test-trigger-is January 22, 2025 09:43 — with GitHub Actions Inactive
r.appendLiteral(old, new)
}

func trimQuotes(s string) string {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is this needed?

If we only care about quoting strings, you can look at fmt.Sprintf with %q.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I care about strings inside JSON that might have been concatenated with others, e.g. xy becomes "XY" in json. Now I need to replace x -> X.

@denik denik added this pull request to the merge queue Jan 22, 2025
Merged via the queue into main with commit fde30ff Jan 22, 2025
@denik denik deleted the denik/sync-root-test branch January 22, 2025 10:23
github-merge-queue bot pushed a commit that referenced this pull request Jan 22, 2025
## Changes
If git is not detected, set default worktree root to sync root.
Otherwise NewFileSet/View raise an error about worktree root being
outside view root in acceptance/bundle/sync-paths-dotdot.

This behavior is introduced in
#1945

Stacked on #2202

## Tests
Existing tests.
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