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.

Sign up for GitHub

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

Jump to bottom

test coverage #24

Closed
3 of 4 tasks
amtoine opened this issue May 8, 2023 · 4 comments · Fixed by #122
Closed
3 of 4 tasks

test coverage #24

amtoine opened this issue May 8, 2023 · 4 comments · Fixed by #122
Assignees
@melMass @amtoine
Labels
ci Something related to the Continuous Integration core Related to the main `nu-git-manager` module enhancement New feature or request help wanted Extra attention is needed sugar Related to the `nu-git-manager-sugar` module tests Something related to the tests of the library

Comments

@amtoine
Copy link
Owner

amtoine commented May 8, 2023 •
edited
Loading

in the long run, i think we need unit and integration tests, as much as possible 👍
@amtoine amtoine added enhancement New feature or request help wanted Extra attention is needed ci Something related to the Continuous Integration tests Something related to the tests of the library labels May 8, 2023
@amtoine amtoine pinned this issue May 8, 2023
@amtoine amtoine mentioned this issue Oct 12, 2023
Merged
melMass pushed a commit that referenced this issue Oct 19, 2023
@amtoine
ci: 💚 Initial CI setup for tests (#27)
4147388 
related to
- nushell/nupm#30
- #24

wait for these to land 
- nushell/nupm#30

## description
this PR
- adds an empty `tests/mod.nu` module so that `nupm test` can work
- adds a CI greatly inspired by nushell/nupm#30
  - does not need the `CWD` trick for Windows (:pray:)
  - pulls down Nushell from nightly builds
  - clones Nupm
  - shows the version of Nushell
  - runs the tests
@amtoine
Copy link
Owner Author

amtoine commented Oct 26, 2023

i updated this issue's tasks to reflect the fact that we need tests for the gm commands themselves, e.g. these kind of tests would help catch bugs like the one with $env.GIT_REPOS_CACHE = /tmp/repos.cache in #39, see the PR for more details 😉

@amtoine amtoine mentioned this issue Oct 27, 2023
Merged
amtoine added a commit that referenced this issue Oct 28, 2023
@amtoine @melMass
extract cache logic out of gm itself (#44)
f9464f3 
as explained in
#24 (comment),
i encountered a cache bug when working on #39 => because the cache path
can now be fully set by the user with `$env.GIT_REPOS_CACHE`, it does
not have the `.nuon` extension in general and thus we cannot assume
`open $env.GIT_REPOS_CACHE` to open the file as NUON data...

## description
in order to allow testing more of the internals of the `gm` commands, i
propose in this PR to move the cache logic as much as possible from `gm`
itself to a new internal `fs/cache.nu` module.

> **Important**
> this might sounds like *clean code*, e.g. where you put all the code
in nested oneliners...
> i would argue it's not the goal of this PR and the fact that the
resulting commands are quite small is simply because they are doing
quite simple stuff!
> the goal here is to move the internal logic out of the public `gm`
which exposes the main API to allow testing as much as possible without
require to `gm clone` actual repositories or `gm remove` in an
interactive manner 😋

### changelog
- move `get-repo-store-cache-path` and `check-cache-file` to a new
`fs/cache.nu` module
- move most of the cache logic to commands in `fs/cache.nu`
- adds tests for the cache functionalities

## tests
a new `cache-manipulation` test in `tests/mod.nu`.

---------

Co-authored-by: Mel Massadian <[email protected]>
@amtoine
Copy link
Owner Author

amtoine commented Oct 28, 2023 •
edited
Loading

What is the link between the two? Sorry I don't see it.

Originally posted by @melMass in #44 (comment)

answering you here directly 😌

the idea behind #44 was that there was cache-related logic hardcoded inside the gm commands themselves.
however because we do not test these commands directly, only the internals of the nu-git-manager library, a bug with the cache was hidden... and then addressed in #39 by not assuming the extension of user-defined caches is always .nuon 👍

i managed to move this logic to the library out of the public API and the gm commands 🥳

the question is:

  • do we keep these tests only and thus close this PR because basically everything is covered now i think?
  • do we write true integration tests for the gm commands and thus leave this PR open?

such tests could look like

gm clone https://github.com/amtoine/nu-git-manager
assert equal (gm list) ["github.com/amtoine/nu-git-manager"]

that kind of stuff 😋

@melMass
Copy link
Collaborator

melMass commented Oct 28, 2023

I would vote for merging this and do a separate PR for testing public commands, I think it makes sense to have test for these too

@amtoine
Copy link
Owner Author

amtoine commented Oct 28, 2023

I would vote for merging this and do a separate PR for testing public commands, I think it makes sense to have test for these too

i've updated the issue description accordingly => integration / end-to-end tests still need to be addressed 😊

@amtoine amtoine mentioned this issue Oct 28, 2023
Merged
@amtoine amtoine added sugar Related to the `nu-git-manager-sugar` module core Related to the main `nu-git-manager` module labels Nov 2, 2023
@amtoine amtoine self-assigned this Nov 4, 2023
amtoine added a commit that referenced this issue Nov 7, 2023
@amtoine
test gm commands end to end (#48)
0a7e901 
related to
- #24 

## description
in order to be able to test the `gm remove` command:
- add `--no-confirm` to `gm remove`

in a new `tests/gm.nu` test module, adds
- a `run-with-env` command to take care of the test environment wrapping
and cleaning
- tests for
  - `gm list`
  - `gm update-cache`
  - `gm status`
  - `gm clone`
  - `gm remove` 
- the true expected errors next to `assert error` in NOTE, for when
we'll be able to check the content of an error in Nushell 😏
@amtoine amtoine mentioned this issue Dec 3, 2023
Merged
amtoine added a commit that referenced this issue Dec 3, 2023
@amtoine
refactor git and git-prompt modules in sugar to allow testing (#…
9359ac6 
…121)

related to
- #24

## description
the `./src/nu-git-manager-sugar/` directory structure changed from
```
.
|-- extra.nu
|-- git-prompt.nu
|-- git.nu
|-- github.nu
`-- mod.nu
```
to
```
.
|-- extra.nu
|-- git
|   |-- lib
|   |   |-- lib.nu
|   |   `-- prompt.nu
|   |-- mod.nu
|   `-- prompt.nu
|-- github.nu
`-- mod.nu
```

that is
- all the Git commands, e.g. `gm repo ...`, are still exposed as before
from `git/mod.nu`
- `git-prompt` has been moved to `git/prompt.nu` and the main _setup_
command is now called `nu-git-manager-sugar git prompt setup`
- the commands from past `git-prompt.nu` that would need to be tested
have been moved to `git/lib/` which is a _private_ module
@amtoine amtoine mentioned this issue Dec 3, 2023
Merged
amtoine added a commit that referenced this issue Dec 13, 2023
@amtoine
test the Git prompt (#122)
b333280 
should
- close #24

## description
this PR
- adds three tests for the three building blocks of the prompt
    - `repo-revision` for `get-revision`
    - `repo-current-action` for `git-action`
    - `build-left-prompt` for `get-left-prompt`
- fixes a bug with `$env.CMD_DURATION` -> when the shell starts its
value is `"0823"`, an easter egg of Nushell 😏
- uses sanitized path in `simplify-path` and sanitize paths in
`get-left-prompt` for Windows
@amtoine amtoine unpinned this issue Jan 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@melMass melMass

@amtoine amtoine

Labels
ci Something related to the Continuous Integration core Related to the main `nu-git-manager` module enhancement New feature or request help wanted Extra attention is needed sugar Related to the `nu-git-manager-sugar` module tests Something related to the tests of the library
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

test the Git prompt amtoine/nu-git-manager
2 participants
@melMass @amtoine