The fix for #578 still not completely fix the branch name

[wustho]

Description

First thanks for #578 fix, but there is an additional issue that comes with the fix, so if I named the branch fix/foo-bar, it gets pushed to remote as only foo-bar, the fix/ is removed.

Neovim version

NVIM v0.10.0-dev-46+ge23c6ebed
Build type: RelWithDebInfo
LuaJIT 2.1.0-beta3

Operating system and version

Kubuntu 22.04.2 LTS x86_64

Steps to reproduce

1. Create a branch that contains slash, eg fix/some-issue
2. Push using P followed by u

Expected behavior

Neogit should push the branch named fix/some-issue with git push origin fix/some-issue

Actual behavior

Neogit pushed the branch named fix/some-issue with git push origin some-issue

Minimal config

{
    'NeogitOrg/neogit', dependencies = 'nvim-lua/plenary.nvim',
    cmd = { 'Neogit' },
    config = function()
      require('neogit').setup({
        signs = {
          -- { CLOSED, OPENED }
          -- section = { '▶', '▼' },
          -- item = { '▶', '▼' },
          section = { '', '' },
          item = { '', '' },
          hunk = { '', '' },
        },
        integrations = {
          diffview = true,
        },
      })
    end,
  }

[ten3roberts]

Thanks for the discovery. Will jump on it in a seconds

[CKolkey]

@ten3roberts I think we've gotta start requiring test coverage for these kind of changes, to confirm that it doesn't break existing behaviour.

[ten3roberts]

Yes, agree. I do think we could set up two git repositories in the testing environment, and have one as a local remote. That way we could test the pushing and pulling parts as well

[CKolkey]

Don't see why not - sounds like an interesting puzzle to solve. It would be a really good idea. Our testing story kinda leaves a lot to be desired.

[CKolkey]

Something else to consider for the testing setup is to actually ignore the git repo entirely - treat the CLI as our API boundary. Since we know what the intended command string is (git push origin master, f.ex), just run an action and assert that process.new got a table with the intended command. That way, there's no reason to fuss about with multiple repos for the majority of the tests

[ten3roberts]

Thats a very good idea.

I suppose if works neatly if git is deterministic and stable. If something in git changes we won't detect that. For example what "git push" would do for a new branch and alike

[CKolkey]

Yeah, thats the trade off mocking. But given the age of git, I'd consider it to be stable enough to depend on. There are even old flags that still work, despite the man page saying not to.

[PriceHiller]

Can I offer an alternative?

Actually use a git repository, but use a local git bare repo.

Here's a dirty secret of git, you can host a "local" repository without a network connection.

Try these commands:

NEOGIT_DIR_REPO="$(mktemp -d)/neogit"
git clone --bare https://github.com/NeogitOrg/neogit.git "${NEOGIT_DIR_REPO}"
pushd "${NEOGIT_DIR_REPO}"

TEST_DIR="$(mktemp -d)/neogit"
git clone "${NEOGIT_DIR_REPO}" "${TEST_DIR}"
pushd "${TEST_DIR}"

That will clone neogit from the new "upstream" locally in a directory. That way you can still run all the relevant git commands without any network connection and be able to easily create and destroy the git repos as needed.

This would make it easy to test Neogit end to end instead of just validating command strings.

[CKolkey]

@treatybreaker I'm definitely into that - feel like taking a swing at it? Our current testing situation leaves a lot to be desired 😅

[PriceHiller]

Yes, I will take a shot at it later today or tomorrow. Currently struggling and dying on the hill of #607.