Skip to content

Explicitly set a static global git configuration while testing#58

Merged
leander-dsouza merged 3 commits intomainfrom
cottsay/global-git-config
Sep 9, 2025
Merged

Explicitly set a static global git configuration while testing#58
leander-dsouza merged 3 commits intomainfrom
cottsay/global-git-config

Conversation

@cottsay
Copy link
Member

@cottsay cottsay commented Sep 8, 2025

Basic Info

Info Please fill out this column
Ticket(s) this addresses N/A
Primary OS tested on Fedora
Is this a breaking change? No
Does this PR contain AI generated software? No

Description of contribution in a few bullet points

Some of the vcs2l tests have expectations that certain git configuration values are set. We should ignore any custom user configurations and explicitly set the configuration values for those variables that are assumed by the tests.

In particular, the captured expected command output doesn't account for git versions which print a hint when defaultBranch isn't explicitly set (as part of the master -> main transition), and there is captured output which contains abbreviated SHA values that are expected to contain 7 characters.

Description of how this change was tested

$ python3 -m pytest test -q
.....................                                                                                                                                                                                                                                 [100%]
21 passed in 19.27s

Some of the vcs2l tests have expectations that certain git configuration
values are set. We should ignore any custom user configurations and
explicitly set the configuration values for those variables that are
assumed by the tests.

Signed-off-by: Scott K Logan <logans@cottsay.net>
@cottsay cottsay self-assigned this Sep 8, 2025
@cottsay cottsay added the bug Something isn't working label Sep 8, 2025
Copy link
Member

@leander-dsouza leander-dsouza left a comment

Choose a reason for hiding this comment

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

Nice :), this is a more robust solution than the workaround I had suggested in #54 (Closing that now).

A suggestion I would like to add is probably prefixing the file with a dot - .gitconfig.

In addition, the initial configuration set for git needs to be removed from the testing workflow over here.

Signed-off-by: Scott K Logan <logans@cottsay.net>
@cottsay
Copy link
Member Author

cottsay commented Sep 9, 2025

Thanks, addressed in 328b653.

claraberendsen
claraberendsen previously approved these changes Sep 9, 2025
Copy link
Contributor

@claraberendsen claraberendsen left a comment

Choose a reason for hiding this comment

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

Thanks for opening this, LGMT!

leander-dsouza
leander-dsouza previously approved these changes Sep 9, 2025
Copy link
Member

@leander-dsouza leander-dsouza left a comment

Choose a reason for hiding this comment

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

Minor nitpick regarding the configuration file, everything else looks good for merge :)

Signed-off-by: Scott K Logan <logans@cottsay.net>

Co-authored-by: Leander Stephen D'Souza <leanderdsouza1234@gmail.com>
Copy link
Member

@leander-dsouza leander-dsouza left a comment

Choose a reason for hiding this comment

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

The tests are known to be a bit flaky on Windows, so they can be ignored for now.

@leander-dsouza leander-dsouza merged commit 964a3fb into main Sep 9, 2025
10 of 14 checks passed
@leander-dsouza leander-dsouza deleted the cottsay/global-git-config branch September 9, 2025 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants