Skip to content

fix(test): support running our test suite under -trimpath, closes #397#439

Merged
klaernie merged 4 commits intomainfrom
trimpath
Mar 11, 2025
Merged

fix(test): support running our test suite under -trimpath, closes #397#439
klaernie merged 4 commits intomainfrom
trimpath

Conversation

@klaernie
Copy link
Copy Markdown
Member

@klaernie klaernie commented Feb 8, 2025

Thanks for @gkampitakis we can now support running the editorconfig-checker test suite with -trimpath.

@klaernie klaernie linked an issue Feb 8, 2025 that may be closed by this pull request
build failure since #360 #397
Closed
@klaernie
Copy link
Copy Markdown
Member Author

klaernie commented Feb 8, 2025

@xsy420, @orhun and @anjandev: I would welcome a review from you confirming that I did not overlook any problems that could occur in your build processes.

@xsy420
Copy link
Copy Markdown

xsy420 commented Feb 8, 2025

it build fine with my PKGBUILD after updating _commit variable to f57718a on my machine
and also in gitlab ci https://gitlab.archlinux.org/xsy420/editorconfig-checker-git/-/pipelines/119244
Thank you all

theoludwig
theoludwig approved these changes Feb 9, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@theoludwig theoludwig left a comment

Choose a reason for hiding this comment

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

Great! 😄

To clarify, does that mean that now editorconfig-checker can be compiled both with and without the -trimpath option? If we don't compile with -trimpath, it works too right?

Should we release a v3.2.1, as soon, as this is merged, to unblock Alpine Linux and other distributions?

@klaernie
Copy link
Copy Markdown
Member Author

klaernie commented Feb 9, 2025

To clarify, does that mean that now editorconfig-checker can be compiled both with and without the -trimpath option? If we don't compile with -trimpath, it works too right?

It will work with both with and without -trimpath.

Should we release a v3.2.1, as soon, as this is merged, to unblock Alpine Linux and other distributions?

Definitely.

@klaernie klaernie requested a review from ccoVeille February 12, 2025 20:52
@klaernie klaernie force-pushed the trimpath branch 3 times, most recently from 13e765e to e202831 Compare March 7, 2025 08:09
@klaernie klaernie mentioned this pull request Mar 7, 2025
Merged
klaernie added 4 commits March 10, 2025 20:52
@klaernie
chore(deps): upgrade go-snaps to v0.5.10 that supports trimpath builds
14eb840
@klaernie
build(test): start running a pass of our testsuite under -trimpath
f0131d7 
This commit is guaranteed to be broken, since our code not yet handles
the caveats regarding the current working dir assumption that go-snaps
now has.
@klaernie
test: echo the commands being executed, to help the user find out whi…
6e516da 
…ch test phase fails
@klaernie
fix(test): ensure snaps finds the snapshot directory by using an abso…
66d2dad 
…lute path

Since we "tamper" with the current working directory to achieve a
certain output from files.GetRelativePath(), we now need to help
snaps find the __snapshots__ directory.
@klaernie
Copy link
Copy Markdown
Member Author

klaernie commented Mar 10, 2025

@mstruebing could I bother you for the final review?

mstruebing
mstruebing approved these changes Mar 11, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@mstruebing mstruebing left a comment

Choose a reason for hiding this comment

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

LGTM

@klaernie klaernie merged commit fc78406 into main Mar 11, 2025
@klaernie klaernie deleted the trimpath branch March 11, 2025 14:54
@editorconfig-checker-bot editorconfig-checker-bot mentioned this pull request Mar 11, 2025
Merged
@BrewTestBot BrewTestBot mentioned this pull request Mar 15, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mstruebing mstruebing mstruebing approved these changes

@theoludwig theoludwig theoludwig approved these changes

@ccoVeille ccoVeille Awaiting requested review from ccoVeille

+1 more reviewer

@orhun orhun orhun approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

build failure since #360

5 participants

@klaernie @xsy420 @mstruebing @orhun @theoludwig