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

GitHub Actions workflow for running Automated Tests #62

Closed
wants to merge 19 commits into from

Commits on Jul 16, 2024

  1. Added in support for MaEVe scp to take in json file

    Signed-off-by: louisg1337 <[email protected]>
    Signed-off-by: Mahadik, Mukul Chandrakant <[email protected]>
    louisg1337 authored and Mahadik, Mukul Chandrakant committed Jul 16, 2024
    Configuration menu
    Copy the full SHA
    d116da5 View commit details
    Browse the repository at this point in the history
  2. Created e2etest.yaml

    Used the existing file template from this PR:
    EVerest#23
    
    --------
    
    Currently testing only with ubuntu-latest OS.
    The demo scripts used are:
    - demo-automated-testing.sh
    
    I have commented out the existing /demo-iso15118-2-ac-plus-ocpp201.sh since as per the Readme.md it requires arguments to be passed and I am unsure which set of arguments need to be passed for the purpose of the workflow.
    https://github.com/EVerest/everest-demo#step-1-run-the-demo
    
    -----
    
    Signed-off-by: Mahadik, Mukul Chandrakant <[email protected]>
    Mahadik, Mukul Chandrakant authored and Mahadik, Mukul Chandrakant committed Jul 16, 2024
    Configuration menu
    Copy the full SHA
    ac37522 View commit details
    Browse the repository at this point in the history
  3. Changed branches to run workflow on

    Not running on main since I will be making code pushes to my feature-branch: automate-tests-actions.
    Also, created a new branch automate-tests-merge that will be used to test workflow runs whenever a PR is created.
    This will simulate creating a PR for merging code changes into main in the parent repo.
    
    Signed-off-by: Mahadik, Mukul Chandrakant <[email protected]>
    Mahadik, Mukul Chandrakant authored and Mahadik, Mukul Chandrakant committed Jul 16, 2024
    Configuration menu
    Copy the full SHA
    1b1b465 View commit details
    Browse the repository at this point in the history
  4. Added 'fi' to end if block in run command

    Signed-off-by: Mahadik, Mukul Chandrakant <[email protected]>
    Mahadik, Mukul Chandrakant authored and Mahadik, Mukul Chandrakant committed Jul 16, 2024
    Configuration menu
    Copy the full SHA
    43289c0 View commit details
    Browse the repository at this point in the history
  5. Adding macos OS to matrix.os list

    Trying out workflow execution for both Ubuntu and MacOS.
    
    Signed-off-by: Mahadik, Mukul Chandrakant <[email protected]>
    Mahadik, Mukul Chandrakant authored and Mahadik, Mukul Chandrakant committed Jul 16, 2024
    Configuration menu
    Copy the full SHA
    fe48996 View commit details
    Browse the repository at this point in the history
  6. Changed macOS version + Added fail-fast property to matrix strategy

    The macOS job was failing as the docker setup action is not suited for macOS ARM64 based images.
    https://github.com/douglascamata/setup-docker-macos-action?tab=readme-ov-file#arm64-processors-m1-m2-m3-series-used-on-macos-14-images-are-unsupported
    
    Hence, now using "macos-latest-large" (macOS 14 as of this commit) image as per the official runner images here:
    https://github.com/actions/runner-images/tree/main
    
    -----------
    
    Also, this failure led to that specific job failing and all other jobs being cancelled including the job for ubuntu OS due to the "fail-fast" property being set to true as default.
    Hence to allow other jobs to go through, setting the fail-fast property to false:
    - https://github.com/orgs/community/discussions/27192#discussioncomment-3254964
    - https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstrategyfail-fast
    
    Signed-off-by: Mahadik, Mukul Chandrakant <[email protected]>
    Mahadik, Mukul Chandrakant authored and Mahadik, Mukul Chandrakant committed Jul 16, 2024
    Configuration menu
    Copy the full SHA
    d8f0a68 View commit details
    Browse the repository at this point in the history
  7. Changed macOS to version 13

    The run failed again but not due to ARM64 architecture but probably due to macos-14 not supported by GitHub action.
    The GitHub action for docker setup on MacOS mentions that only macos-12 and macos-13 versions are supported:
    https://github.com/douglascamata/setup-docker-macos-action#currently-supported-public-runner-images
    
    Hence, trying to change macOS to version 13 now.
    
    Signed-off-by: Mahadik, Mukul Chandrakant <[email protected]>
    Mahadik, Mukul Chandrakant authored and Mahadik, Mukul Chandrakant committed Jul 16, 2024
    Configuration menu
    Copy the full SHA
    5f1a471 View commit details
    Browse the repository at this point in the history
  8. Add a step to print outputs from docker setup GitHub action for MacOS

    Previous run failed yet again at the docker-verify state with error:
    
    ```
      shell: /bin/bash --noprofile --norc -e -o pipefail {0}
    /Users/runner/work/_temp/f2669228-43bc-4f11-9c38-82bd32c1e3b4.sh: line 1: docker: command not found
    Error: Process completed with exit code 127.
    ```
    
    Hence printing outputs as mentioned in the action Readme.md.
    
    -----
    
    Ah! I see the step to install from the docker action was skipped as I had missed changing the macos version to v 13 in the if condition.
    Let's see if it works now.
    
    Signed-off-by: Mahadik, Mukul Chandrakant <[email protected]>
    Mahadik, Mukul Chandrakant authored and Mahadik, Mukul Chandrakant committed Jul 16, 2024
    Configuration menu
    Copy the full SHA
    0034464 View commit details
    Browse the repository at this point in the history
  9. Increasing timeout to 60 minutes

    MacOS docker setup passed finally.
    
    Increasing the timeout since the Downloading and Extracting steps themselves have taken about 20 minutes in the macOS runner.
    But right now even for the demo-automated-tests.sh on MacOS its taking close to 30 minutes just for the download + extraction to complete.
    
    As seen in the commit history, when Shankari changed the timeout to 30 minutes, it was because the demo-iso15118-2-ac script itself was taking close to 30 minutes.
    EVerest@05528d4
    
    Signed-off-by: Mahadik, Mukul Chandrakant <[email protected]>
    Mahadik, Mukul Chandrakant authored and Mahadik, Mukul Chandrakant committed Jul 16, 2024
    Configuration menu
    Copy the full SHA
    548d136 View commit details
    Browse the repository at this point in the history
  10. Adding workflow_dispatch event trigger

    In cicd.yaml:
    - Added my feature branches as well to test triggering of workflows during development. Added a TODO to remove these later.
    - Commented outsteps of docker-build-and-push-images job since it would fail if version TAG is unchanged from existing pushed image.
    - Added a POST request to trigger workflow_dispatch to e2etest.yml via GitHub REST API.
    
    In e2etest.yml:
    - Set the trigger to only workflow_dispatch and removed push, pull_request triggers since we want the tests to run on the latest pushed images only which will happen when first workflow completes successfully.
    - But also to note that the first workflow pushes image only on certain conditions.
    - Need to handle this as well.
    - For now, I've done this by checking if event type is not PR (same check done in cicd.yaml).
    
    Detailed discussion on this in PR for this development.
    
    Signed-off-by: Mahadik, Mukul Chandrakant <[email protected]>
    Mahadik, Mukul Chandrakant authored and Mahadik, Mukul Chandrakant committed Jul 16, 2024
    Configuration menu
    Copy the full SHA
    d30f795 View commit details
    Browse the repository at this point in the history
  11. Added temporary step to ensure workflow doesn't fail

    Signed-off-by: Mahadik, Mukul Chandrakant <[email protected]>
    Mahadik, Mukul Chandrakant authored and Mahadik, Mukul Chandrakant committed Jul 16, 2024
    Configuration menu
    Copy the full SHA
    de9418a View commit details
    Browse the repository at this point in the history
  12. Corrected curl POST request

    Signed-off-by: Mahadik, Mukul Chandrakant <[email protected]>
    Mahadik, Mukul Chandrakant authored and Mahadik, Mukul Chandrakant committed Jul 16, 2024
    Configuration menu
    Copy the full SHA
    c8946e2 View commit details
    Browse the repository at this point in the history
  13. Test commit to trigger workflow

    Previous run was able to dispatch the e2etest workflow but the macos runner didn't have all the tests pass.
    
    Signed-off-by: Mahadik, Mukul Chandrakant <[email protected]>
    Mahadik, Mukul Chandrakant authored and Mahadik, Mukul Chandrakant committed Jul 16, 2024
    Configuration menu
    Copy the full SHA
    c17b1c7 View commit details
    Browse the repository at this point in the history
  14. Test commit to trigger workflow

    MacOS runner abruptly failed in the docker setup step just after 7 minutes.
    
    Signed-off-by: Mahadik, Mukul Chandrakant <[email protected]>
    Mahadik, Mukul Chandrakant authored and Mahadik, Mukul Chandrakant committed Jul 16, 2024
    Configuration menu
    Copy the full SHA
    8d1e4d4 View commit details
    Browse the repository at this point in the history
  15. Fix for failure pointed by Codacy for using ${{...}}

    Failure message:
    ```
    Using variable interpolation `${{...}}` with `github` context data in a `run:` step could allow an attacker to inject their own code into the runner.
    ```
    
    Found a fix which mentions using environment variables instead of directly accessing Github context variables directly in executable statements like the `run` statement:
    cisagov/client-cert-update#53 (comment)
    
    Signed-off-by: Mahadik, Mukul Chandrakant <[email protected]>
    Mahadik, Mukul Chandrakant authored and Mahadik, Mukul Chandrakant committed Jul 16, 2024
    Configuration menu
    Copy the full SHA
    0da20f6 View commit details
    Browse the repository at this point in the history
  16. Removed e2etest.yaml

    ------
    
    Created e2etest.yaml
    
    Used the existing file template from this PR:
    EVerest#23
    
    --------
    
    Currently testing only with ubuntu-latest OS.
    The demo scripts used are:
    - demo-automated-testing.sh
    
    I have commented out the existing /demo-iso15118-2-ac-plus-ocpp201.sh since as per the Readme.md it requires arguments to be passed and I am unsure which set of arguments need to be passed for the purpose of the workflow.
    https://github.com/EVerest/everest-demo#step-1-run-the-demo
    
    -----
    
    Signed-off-by: Mahadik, Mukul Chandrakant <[email protected]>
    Mahadik, Mukul Chandrakant authored and Mahadik, Mukul Chandrakant committed Jul 16, 2024
    Configuration menu
    Copy the full SHA
    248a2e1 View commit details
    Browse the repository at this point in the history
  17. Latest fixes to ensure image built locally and tests run on that imag…

    …e + Added exit_code
    
    Docker compose should now use locally built image.
    Problem was with ${DEMO_DIR}. Directly using docker file name works: ${DEMO_COMPOSE_FILE_NAME}.
    
    Also added exit_code and abort-container so that the docker compose up command exits whenever any one container exits. In our testing case, when manager container exits.
    
    Signed-off-by: Mahadik, Mukul Chandrakant <[email protected]>
    Mahadik, Mukul Chandrakant authored and Mahadik, Mukul Chandrakant committed Jul 16, 2024
    Configuration menu
    Copy the full SHA
    3206739 View commit details
    Browse the repository at this point in the history
  18. Directly using docker compose to run the automated tests

    Was incorrectly using the demo-automated-testing.sh. No need to modify the docker compose directories, file names in there.
    As noted in this comment by Shankari:
    link: EVerest#62 (comment)
    
    > The demo scripts are designed to be "single line demos" that people can run without having to check out any code
    
    Signed-off-by: Mahadik, Mukul Chandrakant <[email protected]>
    Mahadik, Mukul Chandrakant authored and Mahadik, Mukul Chandrakant committed Jul 16, 2024
    Configuration menu
    Copy the full SHA
    532cd41 View commit details
    Browse the repository at this point in the history
  19. Adding the automated test step in the docker build push job itself.

    Using “test before push” workflow mentioned in docker build push action official documentation.
    Link: https://docs.docker.com/build/ci/github-actions/test-before-push/
    
    ---
    
    While this would work, a problem is that I need to put the docker compose step in the docker build push job.
    This means, it would run for all matrix jobs.
    
    In our case currently manager service image takes time to build, and manager depends on mqtt service image which is built pretty quickly. So we are good here.
    
    But consider this scenario.
    What if a matrix job B is dependent on matrix job A but matrix job A hasn’t finished building it’s image as yet, then matrix job B might use the older image which it would have pulled and not the locally built / updated image.
    In this case, the tests would be run on an older image, and could falsely pass the tests.
    
    ---
    
    For instance, in this test workflow run, I wanted to test the success / failure test case and used a different test script. This is copied over in the manager/Dockerfile.
    But since the automated tests step is part of the same matrix job, it would run for all services and not just manager which has the entry point (run-test.sh) script for automated tests.
    
    So, mqtt service image, in this case, also executed the docker compose command, it wasn’t able to find the test script in the manager image.
    The reason is that while I did include the test script in the manager/Dockerfile, this image is still being built, hence latest changes not yet available.
    So, the matrix job for the mqtt service uses the pulled image layers from the cache for the manager image (it pulled in the first place since the manager image was used as a part of the docker compose), and this pulled image did not have the test script file.
    
    Hence the workflow failed saying, no such file found:
    https://github.com/MukuFlash03/everest-demo/actions/runs/9960438276/job/27519676597
    
    ----
    
    One workaround I see then is to run the automated tests conditionally only for the manager service matrix job.
    
    Signed-off-by: Mahadik, Mukul Chandrakant <[email protected]>
    Mahadik, Mukul Chandrakant authored and Mahadik, Mukul Chandrakant committed Jul 16, 2024
    Configuration menu
    Copy the full SHA
    b7b24e2 View commit details
    Browse the repository at this point in the history