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

Set working directory in lazygit test command #3215

Merged
merged 2 commits into from
Jan 12, 2024

Conversation

jesseduffield
Copy link
Owner

@jesseduffield jesseduffield commented Jan 12, 2024

We need to fetch our list of tests both outside of our test binary and within. We need to get the list from within so that we can run the code that drives the test and runs assertions. To get the list of tests we need to know where the root of the lazygit repo is, given that the tests live in files under that root.

So far, we've used this GetLazyRootDirectory() function for that, but it assumes that we're not in a test directory (it just looks for the first .git dir it can find). Because we didn't want to properly fix this before, we've been setting the working directory of the test command to the lazygit root, and using the --path CLI arg to override it when the test itself ran. This was a terrible hack.

Now, we're passing the lazygit root directory as an env var to the integration test, so that we can set the working directory to the actual path of the test repo; removing the need to use the --path arg.

  • PR Description

  • Please check if the PR fulfills these requirements

  • Cheatsheets are up-to-date (run go generate ./...)
  • Code has been formatted (see here)
  • Tests have been added/updated (see here for the integration test guide)
  • Text is internationalised (see here)
  • Docs (specifically docs/Config.md) have been updated if necessary
  • You've read through your own file changes for silly mistakes etc

@jesseduffield jesseduffield added the maintenance For refactorings, CI changes, tests, version bumping, etc label Jan 12, 2024
Copy link

codacy-production bot commented Jan 12, 2024

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
-0.01% (target: -2.00%) 91.89%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (5c888a0) 45840 37731 82.31%
Head commit (a1ce602) 45847 (+7) 37733 (+2) 82.30% (-0.01%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#3215) 37 34 91.89%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

You may notice some variations in coverage metrics with the latest Coverage engine update. For more details, visit the documentation

We need to fetch our list of tests both outside of our test binary and within. We need
to get the list from within so that we can run the code that drives the test and runs
assertions. To get the list of tests we need to know where the root of the lazygit repo
is, given that the tests live in files under that root.

So far, we've used this GetLazyRootDirectory() function for that, but it assumes that
we're not in a test directory (it just looks for the first .git dir it can find). Because
we didn't want to properly fix this before, we've been setting the working directory of
the test command to the lazygit root, and using the --path CLI arg to override it when
the test itself ran. This was a terrible hack.

Now, we're passing the lazygit root directory as an env var to the integration test, so
that we can set the working directory to the actual path of the test repo; removing the
need to use the --path arg.
@jesseduffield jesseduffield force-pushed the set-working-directory-in-integration-tests branch from 5c6d22f to 8c71618 Compare January 12, 2024 08:59
@jesseduffield jesseduffield force-pushed the set-working-directory-in-integration-tests branch from 14b8f98 to 6c4ca54 Compare January 12, 2024 09:15
For some bizarre reason `pkg/integration/tests/filter_by_path/cli_arg.go` is failing as of 8c71618 like so:

```
test_lazygit

  Usage:
    test_lazygit [git-arg]

  Positional Variables:
    git-arg   Panel to focus upon opening lazygit. Accepted values (based on git terminology): status, branch, log, stash. Ignored if --filter arg is passed.
  Flags:
    -h --help               Displays help with available flag, subcommand, and positional value parameters.
    -p --path               Path of git repo. (equivalent to --work-tree=<path> --git-dir=<path>/.git/)
    -f --filter             Path to filter on in `git log -- <path>`. When in filter mode, the commits, reflog, and stash are filtered based on the given path, and some operations are restricted
    -v --version            Print the current version
    -d --debug              Run in debug mode with logging (see --logs flag below). Use the LOG_LEVEL env var to set the log level (debug/info/warn/error) (default: false)
    -l --logs               Tail lazygit logs (intended to be used when `lazygit --debug` is called in a separate terminal tab)
    -c --config             Print the default config
    -cd --print-config-dir   Print the config directory
    -ucd --use-config-dir     override default config directory with provided directory
    -w --work-tree          equivalent of the --work-tree git argument
    -g --git-dir            equivalent of the --git-dir git argument
    -ucf --use-config-file    Comma separated list to custom config file(s)

Unknown arguments supplied:  filterFile
```

where the CLI args are:
```
([]string) (len=5 cap=5) {
 (string) (len=25) "/tmp/lazygit/test_lazygit",
 (string) (len=6) "-debug",
 (string) (len=108) "--use-config-dir=/Users/jesseduffieldduffield/repos/lazygit/test/_results/filter_by_path/cli_arg/used_config",
 (string) (len=2) "-f",
 (string) (len=10) "filterFile"
}
```

This appears to be a bug in flaggy itself. I've updated to the latest version but it still breaks. Bizarrely it works fine on CI and
only fails locally. Running lazygit locally with `lg -f pkg/gui/controllers/helpers/refresh_helper.go` it works fine. So I don't
know what's going on there. At any rate, I'm just going to get the test passing by passing `-f=filterFile` as a single argument.
@jesseduffield jesseduffield force-pushed the set-working-directory-in-integration-tests branch from 6c4ca54 to a1ce602 Compare January 12, 2024 09:19
@jesseduffield
Copy link
Owner Author

I'm just gonna merge this because it's low stakes if it has issues

@jesseduffield jesseduffield merged commit e9962b9 into master Jan 12, 2024
17 checks passed
@jesseduffield jesseduffield deleted the set-working-directory-in-integration-tests branch January 12, 2024 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance For refactorings, CI changes, tests, version bumping, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant