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

fix: Unit Test Requires "test" Binary #266

Merged
merged 1 commit into from
Aug 25, 2021

Conversation

CoshUS
Copy link
Contributor

@CoshUS CoshUS commented Aug 25, 2021

Issue

The following tests do not mock path resolution which requires the system to have the binary "test".

  • tests/unit/test_workflow.py::TestBaseWorkflow_run::test_must_raise_if_action_crashed
  • tests/unit/test_workflow.py::TestBaseWorkflow_run::test_must_raise_if_action_failed
  • tests/unit/test_workflow.py::TestBaseWorkflow_run::test_must_raise_with_no_actions
  • tests/unit/test_workflow.py::TestBaseWorkflow_run::test_supply_executable_path

Error:

self = Workflow=MyWorkflow
Actions=
        <Mock name='mock.action1' id='1800480812144'>
        <Mock name='mock.action2' id='1800480812384'>
        <Mock name='mock.action3' id='1800480812240'>
args = (), kwargs = {}, valid_paths = {}, invalid_paths = {'test': []}, binary = 'test', binary_checker = <aws_lambda_builders.binary_path.BinaryPath object at 0x000001A33504EE50>

    @functools.wraps(func)
    def wrapper(self, *args, **kwargs):
        valid_paths = {}
        invalid_paths = {}
        # NOTE: we need to access binaries to get paths and resolvers, before validating.
        for binary, binary_checker in self.binaries.items():
            invalid_paths[binary] = []
            try:
                exec_paths = (
                    binary_checker.resolver.exec_paths
                    if not binary_checker.path_provided
                    else binary_checker.binary_path
                )
            except ValueError as ex:
>               raise WorkflowFailedError(workflow_name=self.NAME, action_name="Resolver", reason=str(ex))
E               aws_lambda_builders.exceptions.WorkflowFailedError: MyWorkflow:Resolver - Path resolution for runtime: runtime of binary: test was not successful

This is fine for AppVeyor as "test" exists for Unix systems or Windows with Git installed in a certain way. However, on systems without "test" binary, unit tests will fail.

Get-Command test
CommandType     Name                                               Version    Source                                                                                                                                                 
-----------     ----                                               -------    ------                                                                                                                                                 
Application     test.exe                                           0.0.0.0    C:\Program Files\Git\usr\bin\test.exe   

Description of changes:
Mocks all binary verification for workflow to better adhere to unit testing principles.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@sriram-mv sriram-mv merged commit 262d02f into aws:develop Aug 25, 2021
sriram-mv added a commit that referenced this pull request Aug 31, 2021
* fix: improve support for pep517 builds (#265)

* A pep517 build can declare build dependencies. Pip will then
know to install these dependencies before trying to build a wheel
file.

* When creating a build environment, it's only guaranteed to last
for the duration of the build process. It's not accessible once
a pip command finishes running.

* When we try to retrieve the version of a package we run a "modified"
form of "python setup.py egg_info".

* The problem with this is that we're not using the build environment
that has all the build dependencies installed (it's already gone),
so if setup.py imports a module (e.g. cython) because it expects
it to be there because it declared it as a build dependency
the egg_info command will fail.

* We don't check the RC or have a fallback case if we can't generate
egg info.

* We fail with an indecipherable IndexError.

We now have a fallback where if we can't import/run the setup.py file,
we assume the PKG-INFO file should be in the top level directory of the
sdist so we check if it's there, and if so we use that file.

* Fixed Unit Test Requiring "test" Binary (#266)

* fix(go version parts): remove alphabets from the version for validation (#259)

* fix(go version parts): remove alphabets from the version for validation

- Go versions like 1.12rc1 or 1.16beta1 are supported.
- Test added.

* fix: use regex for go versions

* chore: aws lambda builders version set to 1.7.0 (#269)

Co-authored-by: Cosh_ <[email protected]>
mndeveci added a commit that referenced this pull request Sep 29, 2021
* fix: improve support for pep517 builds (#265)

* A pep517 build can declare build dependencies. Pip will then
know to install these dependencies before trying to build a wheel
file.

* When creating a build environment, it's only guaranteed to last
for the duration of the build process. It's not accessible once
a pip command finishes running.

* When we try to retrieve the version of a package we run a "modified"
form of "python setup.py egg_info".

* The problem with this is that we're not using the build environment
that has all the build dependencies installed (it's already gone),
so if setup.py imports a module (e.g. cython) because it expects
it to be there because it declared it as a build dependency
the egg_info command will fail.

* We don't check the RC or have a fallback case if we can't generate
egg info.

* We fail with an indecipherable IndexError.

We now have a fallback where if we can't import/run the setup.py file,
we assume the PKG-INFO file should be in the top level directory of the
sdist so we check if it's there, and if so we use that file.

* Fixed Unit Test Requiring "test" Binary (#266)

* fix(go version parts): remove alphabets from the version for validation (#259)

* fix(go version parts): remove alphabets from the version for validation

- Go versions like 1.12rc1 or 1.16beta1 are supported.
- Test added.

* fix: use regex for go versions

* chore: aws lambda builders version set to 1.7.0 (#269)

* test: Temporarily disabling two Python integration tests failing on Windows (#282)

* feat: Allow Node.js projects to be built without requiring a package.json (#284)

* Allow nodejs to build without requiring a manifest

* Fix existing tests

* Remove expected .package-lock

* Define actions in the scope they're used

* feat: ARM support (#25) (#287)

Co-authored-by: Mathieu Grandis <[email protected]>

* chore: bump version to 1.8.0 (#288)

* fix: remove rendundant get_go_versions method (#290)

Co-authored-by: Sriram Madapusi Vasudevan <[email protected]>
Co-authored-by: Cosh_ <[email protected]>
Co-authored-by: Mathieu Grandis <[email protected]>
Co-authored-by: Daniel Mil <[email protected]>
CoshUS added a commit that referenced this pull request Sep 29, 2021
* fix: improve support for pep517 builds (#265)

* A pep517 build can declare build dependencies. Pip will then
know to install these dependencies before trying to build a wheel
file.

* When creating a build environment, it's only guaranteed to last
for the duration of the build process. It's not accessible once
a pip command finishes running.

* When we try to retrieve the version of a package we run a "modified"
form of "python setup.py egg_info".

* The problem with this is that we're not using the build environment
that has all the build dependencies installed (it's already gone),
so if setup.py imports a module (e.g. cython) because it expects
it to be there because it declared it as a build dependency
the egg_info command will fail.

* We don't check the RC or have a fallback case if we can't generate
egg info.

* We fail with an indecipherable IndexError.

We now have a fallback where if we can't import/run the setup.py file,
we assume the PKG-INFO file should be in the top level directory of the
sdist so we check if it's there, and if so we use that file.

* Fixed Unit Test Requiring "test" Binary (#266)

* fix(go version parts): remove alphabets from the version for validation (#259)

* fix(go version parts): remove alphabets from the version for validation

- Go versions like 1.12rc1 or 1.16beta1 are supported.
- Test added.

* fix: use regex for go versions

* chore: aws lambda builders version set to 1.7.0 (#269)

* test: Temporarily disabling two Python integration tests failing on Windows (#282)

* feat: Allow Node.js projects to be built without requiring a package.json (#284)

* Allow nodejs to build without requiring a manifest

* Fix existing tests

* Remove expected .package-lock

* Define actions in the scope they're used

* feat: ARM support (#25) (#287)

Co-authored-by: Mathieu Grandis <[email protected]>

* chore: bump version to 1.8.0 (#288)

* fix: remove rendundant get_go_versions method (#290)

Co-authored-by: Sriram Madapusi Vasudevan <[email protected]>
Co-authored-by: Cosh_ <[email protected]>
Co-authored-by: Mathieu Grandis <[email protected]>
Co-authored-by: Daniel Mil <[email protected]>

Co-authored-by: Sriram Madapusi Vasudevan <[email protected]>
Co-authored-by: Cosh_ <[email protected]>
Co-authored-by: Mathieu Grandis <[email protected]>
Co-authored-by: Daniel Mil <[email protected]>
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 this pull request may close these issues.

3 participants