Update Bintray Package Info

[uilianries]

Hi!

This PR is about a plugin to update Bintray package page.

For each new package created, we need to fill manually Bintray page, but all needed data could be extract from the recipe.

Related issue: bincrafters/community#476

[danimtb]

I really like the idea of this new plugin as Bintray still does not fill metadata automatically, but we will have to wait to make a decision on how plugins will be managed for installation, versioning and activation and what will be the long-term purpose of repository. Please, keep the PR open and I will try to discuss it with the team. Many thanks!!!

[jgsogo]

We have added Travis and AppVeyor CI to this repo and an example test for the attribute_checker hook. We need to merge master into this PR (to get CI) and then implement some tests to check that the hook is working (it should be easy to mock requests.get function, but ping me if you need help).

Thanks!

[uilianries]

@jgsogo the bintray hook is updated!

Since it requires bintray api and interface validation I would prefer to create a fake package to validate all steps:

https://bintray.com/conan-community/test-distribution/foo%3Abar

You will need to set CONAN_LOGIN_USERNAME_VIRTUAL and CONAN_PASSWORD_VIRTUAL. Both values are the same used for conan community login.

TIL: If you need to get an environment variable when running tox, just use passenv instead of spending 2h trying all kind of workaround ...

[uilianries]

Ops!

ERROR: [HOOK - conan-center.py] post_build(): 'settings.os' doesn't exist
'settings' possible configurations are ['arch_build', 'os_build']
Traceback (most recent call last):
  File "/opt/miniconda3/lib/python3.6/site-packages/conans/client/hook_manager.py", line 56, in execute
    method(output, **kwargs)
  File "/home/uilian/.conan/hooks/conan-center.py", line 121, in post_build
    if not _files_match_settings(conanfile, conanfile.build_folder):
  File "/home/uilian/.conan/hooks/conan-center.py", line 209, in _files_match_settings
    if conanfile.settings.os == "Windows":
  File "/opt/miniconda3/lib/python3.6/site-packages/conans/model/settings.py", line 257, in __getattr__
    self._check_field(field)
  File "/opt/miniconda3/lib/python3.6/site-packages/conans/model/settings.py", line 253, in _check_field
    raise undefined_field(self._name, field, self.fields, self._parent_value)
conans.errors.ConanException: 'settings.os' doesn't exist
'settings' possible configurations are ['arch_build', 'os_build']

I need to support installer packages too.

[jgsogo]

Minor changes related to the renaming from plugins to hooks.

[jgsogo]

Having a look at this PR I have some doubts about the test suite. Do you think it is ok to call Conan CLI or shall we just test the hook function with mocked arguments?

In the beginning, I thought that it would be ok to use Conan CLI, this way we can test our hooks against different versions of Conan itself (it is what I did with the attribute_checker hook), but now I see that this approach has several drawbacks in order to mock functions: as we are running an external tool through command line, this is no longer our program and we cannot mock function calls :/, so:

1. we can use conan_api (undocumented, not stable) to avoid using CLI, or
2. we can test just the hook function using fake arguments.

Each approach has pros and cons:

1. conan_api is not stable and it might be modified, we can encapsulate these modifications inside a helper conan client that we will need to maintain and it will get more and more confusing; but we will be sure that all the Conan workflow will be taken into account for our hooks.

   We will need to run all tests against all (supported) versions of Conan, like it is implemented now.

2. Faster, easier and safer, nothing external to the hook itself will affect the tests. But to keep consistency and check that every Conan version will work, we would need some extra tests, calling the Conan CLI to check that the parameters that get to the hooks functions are the expected ones (the ones we will be mocking in the actual tests).

   No need to run the tests related to the hooks against different Conan versions, only the tests which check that the arguments that reach the hooks functions are the expected ones.

In any case, I think that we should mock the calls to external services, we can store sample json responses (also failing ones) and our test suite won't rely on Bintray status.
What about if Bintray changes its API? Well, I suppose it won't happen too often, but we can run tests against Bintray API and check our stored jsons against the returned ones, this way we will know if a failing test is due to our code or to Bintray changes.

What do you think about this dissertation?

[uilianries]

I really appreciate your feedback! I thought in both ideas before.

I've Conan API used for many scripts and Bincrafters' projects, but as you said, it will be changed often.

I like the second option, add mocks, sounds more "clean", but I don't know how to make it correctly. This hook needs to make requests to Bintray, change all metadata and check the results. If we are trying to make an integration test, so we couldn't not change the client side I think, we should be able to mock bintray side only. However, we would need to create fake answers based on Bintray and Conan REST API, but it sounds like creating a framework just to check a small piece of code.

Maybe, instead to run requests directly, we could use some part of Conan base code (Uploader class, just a guess), with an well known mock supported from Conan tests.

[jgsogo]

Talking about second option: testing just the hook function with mocked arguments

• we can mock requests.get function with something like (this is not going to work, I haven't tested it):

  from mock import patch
  ...
  
  def side_effect(url, ...):
     # Read proper file depending on URL passed as argument
     return file_json
  
  # Prepare the arguments for the hook
  # Mock the requests object
  with patch.object("requests.get", side_effect= side_effect):
     # Run the hook function
     post_upload(arguments,...)

There are still integration tests:

• [Required] run a single test for every conan version (like it is implemented now in the CI), using a test_framework_hook that checks that the inputs for every hook are the expected
• [Optional] check that the json returned from Bintray and the one stored in the files that we are using to mock requests.get are equal.

[uilianries]

@jgsogo I made some changes following your points. Thanks for the example, it helped me to understand how to mock requests 👍

Okay, the new test calls the hook directly, and all Bintray's answers are mocked up. I've tried to follow Bintray status code and messages -- All commands were executed to check Bintray's response.

[jgsogo]

It looks very good, just a couple of comments... and it is on my side to refactor the test suite ^^

[uilianries]

@jgsogo I've updated the unit tests, but as hooks doesn't have __init__.py, I've added hooks dir in the path: https://github.com/conan-io/hooks/pull/2/files#diff-8eebc7307c1e61c5efaea1449796443cR19

[uilianries]

Up! Should we wait for #16 before to merge this PR?

[jgsogo]

yes, #16 is the top priority now in this repo. I don't if #16 will get merged, but that discussion should be resolved before merging anything else (tests could be completely different)

[jgsogo]

Hi @uilianries, we really want to start promoting hooks and this one can be very interesting. Although I think that testing will follow the approach in #17 (more or less like it is now) maybe it is better to split this PR into two: the hook itself (to be merged as soon as possible), and the testing part (so we can comment about it).

Also, please add a little documentation in the README.md, we are not sure if that will be the final place for documentation, but at least it will be the first place.

Thanks

[uilianries]

Understood, I gonna do it today 😸

[jgsogo]

Looks good!

[jgsogo]

👍 Waiting for CI

[uilianries]

Thanks Javi!

[uilianries]

@jgsogo All green!

[jgsogo]

Waiting for @danimtb to merge it.