P1sec / pycrate

A Python library to ease the development of encoders and decoders for various protocols and file formats; contains ASN.1 and CSN.1 compilers.
GNU Lesser General Public License v2.1
381 stars 132 forks source link

CI/CD Workflow for test/packaging #137

Closed benmaddison closed 3 years ago

benmaddison commented 3 years ago

resolves #135

benmaddison commented 3 years ago

the CI workflow can be seen in action on my fork: https://github.com/benmaddison/pycrate/actions

benmaddison commented 3 years ago

Summary of changes

Pre-merge steps

The publish job requires that there are API tokens for test.pypi.org and pypi.org saved as "secrets" on the gihub repo, named TEST_PYPI_TOKEN and PYPI_TOKEN.

The tokens can be generated at:

The corresponding secrets can be created at: https://github.com/P1sec/pycrate/settings/secrets/actions

This should be completed before the PR is merged/pushed, otherwise the initial CI run will fail.

Post-merge steps

I suggest that a "pre-release" 0.5.0b1 version is tagged and pushed, so that the publish job can be tested without burning a full release number.

Note that the project currently has hard-coded version numbers in setup.py, and the version number on pypi.org with be taken from there, not from the name of the tag.

I recommend looking at the setuptools_scm plug-in to enable version numbering based on git tags.

Please let me know if you have questions, or would like any changes.

p1-bmu commented 3 years ago

I merged the PR, thanks for this. For the version, I prefer dealing myself with a specific VERSION file at the root instead of relying on any plugins or dependencies. Regarding the dependencies described in the [testenv] section of the new setup.cfg file, I will remove them, as they are not required for most of the use of the library (neither for running the test vectors).

benmaddison commented 3 years ago

I merged the PR, thanks for this.

You're welcome. Thanks for accepting and merging so fast.

For the version, I prefer dealing myself with a specific VERSION file at the root instead of relying on any plugins or dependencies.

Fair enough, your call!

Regarding the dependencies described in the [testenv] section of the new setup.cfg file, I will remove them, as they are not required for most of the use of the library (neither for running the test vectors).

I would not recommend this.

The [testenv] section is only used by tox to set up the test environment. Those dependencies will not be installed automatically (via pip install ..., python setup.py install, etc) unless the appropriate "extra" is selected.

If you remove the dependencies from the test environment, then:

I notice that the initial workflow run failed at the publish job. Did you create the secrets for the PyPI tokens?

p1-bmu commented 3 years ago

OK, it seems tox is unable to handle reading the VERSION file I just set at the root...

Processing ./.tox/.tmp/package/1/pycrate-0.4.1.zip
    ERROR: Command errored out with exit status 1:
     command: /home/runner/work/pycrate/pycrate/.tox/py38-linux/bin/python -c 'import io, os, sys, setuptools, tokenize; sys.argv[0] = '"'"'/tmp/pip-req-build-g8xgxbmh/setup.py'"'"'; __file__='"'"'/tmp/pip-req-build-g8xgxbmh/setup.py'"'"';f = getattr(tokenize, '"'"'open'"'"', open)(__file__) if os.path.exists(__file__) else io.StringIO('"'"'from setuptools import setup; setup()'"'"');code = f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' egg_info --egg-base /tmp/pip-pip-egg-info-gqh7qic3
         cwd: /tmp/pip-req-build-g8xgxbmh/
    Complete output (5 lines):
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/tmp/pip-req-build-g8xgxbmh/setup.py", line 10, in <module>
        with open(os.path.join(os.path.dirname(__file__), "VERSION")) as fd:
    FileNotFoundError: [Errno 2] No such file or directory: '/tmp/pip-req-build-g8xgxbmh/VERSION'

Regarding the dependencies, as I have no test vectors that require them, I prefer to remove them. This is useless to require the github CI-CD to install them each time it runs, when it's not needed.

i also copied again the 2 pypy tokens, after I got a complain that one of it did not start with pypi- (I may have failed the first copy paste ?).

benmaddison commented 3 years ago

OK, it seems tox is unable to handle reading the VERSION file I just set at the root...

The issue is not tox specific: tox creates an sdist and then installs it into the test venv. The issue is that VERSION is not being added to the sdist. You will need a MANIFEST.in file to tell setuptools to add it.

See the following links for some further details: https://packaging.python.org/guides/single-sourcing-package-version/ https://packaging.python.org/guides/using-manifest-in/

Regarding the dependencies, as I have no test vectors that require them, I prefer to remove them. This is useless to require the github CI-CD to install them each time it runs, when it's not needed.

The output from the test suite suggests that they are required for the NAS tests?

When I test without the dependencies installed, I get:

warning: CryptoMobile Python module not found, unable to handle LTE NAS security
warning: CryptoMobile Python module not found, unable to handle 5G NAS security

Is that incorrect?

i also copied again the 2 pypy tokens, after I got a complain that one of it did not start with pypi- (I may have failed the first copy paste ?).

OK, cool. I can't see which secrets are configured. Hopefully it works once the test job is fixed.

p1-bmu commented 3 years ago

OK, it seems it finally worked. The package has been published in https://test.pypi.org/project/pycrate/.

p1-bmu commented 3 years ago

I removed the optional dependencies for testing in the CICD github actions. I am now facing an error with tox which is unable to run the test suite on both Ubuntu and macos, while everything run fine on my side: https://github.com/P1sec/pycrate/actions/runs/850268767

It fails with:

AttributeError: 'module' object has no attribute 'test_pycrate'

It's like it cannot find the test/test_pycrate.py script to run it ; while the command-line python -m unittest test.test_pycrate works nicely. Do you have any idea why tox is unhappy here ?