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

Discuss: add the python project attribute to git extras repository #1102

Closed
vanpipy opened this issue Nov 3, 2023 · 6 comments · Fixed by #1121
Closed

Discuss: add the python project attribute to git extras repository #1102

vanpipy opened this issue Nov 3, 2023 · 6 comments · Fixed by #1121

Comments

@vanpipy
Copy link
Collaborator

vanpipy commented Nov 3, 2023

I want to enhance the dependency management of the python for git extras via poetry, but I don't know it make sense or just a big hammer at this moment, because an unknown dependency will be added at sometime and I think it not a thing need to announce to users or devs. Dependency installed then used, ok, done.

For now, the git-browse needs a mocking context which includes some commands like open, start or others. So I try to do some implements about it until the testpath was found. And I want to add it in local dependency config and do not want to add more unrelative information into readme. This is reason why I create this issue.

@hyperupcall and @spacewander , please take a look here.

@vanpipy
Copy link
Collaborator Author

vanpipy commented Nov 4, 2023

Emm.., I think is useless to add the package manager for git extras repo cause the installation is a problem or just waste the time and the information about the test in the readme should be moved to a new file in the test directory, it is special for the dev who want to create some testcases.

@hyperupcall
Copy link
Collaborator

hyperupcall commented Nov 4, 2023

If we use Python, in my opinion Poetry is a very useful tool to make managing things easier. I didn't mention it before because not everyone uses the tool. If you would like to add it, you certainly have my vote!

I'm not convinced that we need testpath. If we need to mock a command, I think we should do it ourselves. But I'm not the one writing the majority of tests.

I agree with your points that instructions for setting up Python for testing should not be included in the main README.md.

@spacewander
Copy link
Collaborator

Emm.., I think is useless to add the package manager for git extras repo cause the installation is a problem or just waste the time and the information about the test in the readme should be moved to a new file in the test directory, it is special for the dev who want to create some testcases.

Agree with that.

If you would like to add it, you certainly have my vote!

Vote +1 for this too.

@vanpipy
Copy link
Collaborator Author

vanpipy commented Nov 4, 2023

Thanks guys and I will try to restruct the test env for git extras later.

I'm not convinced that we need testpath.

Because of the definition of the git-browse that it will lead the user to a website which display current repository. Normally, the xdg-open can help us to do like that, but the xdg-open is not the part of our functions, we just use it in a special environment, here is linux or unix. I call the part like xdg-open as external API and the external API should be mocked when do unit tests. So I think we should has a mock library to help us. Mocking system commands is a great way to detach the external API for git-browse unit test.

@hyperupcall
Copy link
Collaborator

hyperupcall commented Nov 6, 2023

@vanpipy I don't disagree that we need a way to mock commands, and I did mention:

If we need to mock a command, I think we should do it ourselves.

The simple thing we can do is add a directory, tests/bin to the PATH only when testing, which contains the mocked commands. I think this approach is more clear and approachable for someone not familiar with the codebase; it's an approach that isn't uncommon and is not coupled to a particular library.

@vanpipy
Copy link
Collaborator Author

vanpipy commented Nov 8, 2023

The simple thing we can do is add a directory, tests/bin to the PATH only when testing, which contains the mocked commands. I think this approach is more clear and approachable for someone not familiar with the codebase; it's an approach that isn't uncommon and is not coupled to a particular library.

I got your idea and it is my fault about the mock commands. Let me take some explains.

  • what is the mock commands called and how to be used?
    • It is a temp file or process or only a function.
    • It can be detected used or not.
    • One mocked command works in the python environment.

Once we create the mocking directory and then a python wrapper is needed. It means we need to maintain one more python library. So could the tests/bin satisfy the requirements above easily? It depends cause I review the implement of the testpath did lots of thing I cannot understand for now.

I agree and will to do the tests/bin, but I do not know how many time used. This is why I want to use testpath.

One more, all things we talked about did not affect the git extras itself. Could i misunderstand a particular library?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants