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

Create workflow for languages tests #13

Merged
merged 1 commit into from
Apr 3, 2024

Conversation

AksharP5
Copy link
Collaborator

@AksharP5 AksharP5 commented Mar 27, 2024

ci: test languages in pull requests

---

Closes #12 

Co-Authored-by: Akshar Patel <[email protected]>
Co-Authored-By: Stoney Jackson <[email protected]>

@AksharP5 AksharP5 self-assigned this Mar 27, 2024
@AksharP5 AksharP5 force-pushed the akshar5/run-tests-in-pipeline-a-12 branch 3 times, most recently from 08e56f2 to 32f57e8 Compare March 30, 2024 04:08
Copy link
Member

@StoneyJackson StoneyJackson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good first effort!

with:
image: test-image:latest
run: |
git clone https://github.com/ourPLCC/languages.git
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will always test against the main branch, even when pushing code to a feature branch.

The current working directory should contain the languages code from the feature branch. So we should mount it into the container when we run it. Try something like this...

      -
        name: Test Languages
        uses: addnab/docker-run-action@v3
        with:
          image: test-image:latest
          options: -v ${{ github.workspace }}:/work
          run: |
            /work/bin/test.bash

Copy link
Collaborator Author

@AksharP5 AksharP5 Mar 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you were to use that code without first switching into languages, the plcc repo and its directories appear, for example the installers, src, tests, etc, so you cannot run bin/test.bash. So you first need to checkout languages before testing, but you also need to be on the current feature branch, which can be done through github.head_ref, and now if you run the above code, the next issue is the naming of the container being "work". If you leave it as "work", the tests will not be able to cd properly from setup(), because the path must include "languages" and I have tried to remove that condition, but two tests, OBJ and V3 cd into a different location that includes their names, which is an issue. So I changed the name of "work" to "languages," and tested it again, and now the issue is that there seems to be an issue creating the Java directory from plcc, as the error message is Java: error creating destination subdirectory after each test tries to use plcc or plccmk. This is the part where I am stuck at now, I am not sure how to troubleshoot what exactly is going wrong as the code in plcc.py catches the error and outputs that default message.

    try:
        os.mkdir(dst)
        debug('[lexFinishUp] ' + dst + ': destination subdirectory created')
    except FileExistsError:
        debug('[lexFinishUp] ' + dst + ': destination subdirectory exists')
    except:
        death(dst + ': error creating destination subdirectory')

Any thoughts? @StoneyJackson

run: |
git clone https://github.com/ourPLCC/languages.git
./languages/bin/test.bash
rm -rf languages
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to clean up. All of this is thrown away after the test completes (success or failure).

@AksharP5 AksharP5 force-pushed the akshar5/run-tests-in-pipeline-a-12 branch 5 times, most recently from 3b969c3 to 9a49bc6 Compare March 31, 2024 03:40
@StoneyJackson StoneyJackson marked this pull request as draft March 31, 2024 14:32
@StoneyJackson
Copy link
Member

@AksharP5

Ok... I think this is closer to what we want.

Now the problem is that the tests cannot (and should not) modify any files in the project folder. They should be using a temporary directory outside the project. I think the tests in ourPLCC/plcc may be doing this.

Normally I would say that we should open a different PR for this. But this PR is giving us a good test for this change.

@StoneyJackson
Copy link
Member

I think we're doing this wrong...

The tests in ourPLCC/languages pipeline should not be dependent on the test infrastructure of ourPLCC/plcc. By running our tests inside ourPLCC/plcc's container, we are using its bats. If they remove bats or change its version, then our pipeline may break.

Instead, I think we need to setup our own test environment and run our tests in that environment. In the test environment we install bats and PLCC, and we load up our languages. Then we run our tests.

(Although, one way to "install" PLCC is to use a prebuilt image from ourPLCC/plcc. But this would require a docker-in-docker environment. This could get complicated.)

So... let's define a Dockerfile for the test environment. Then the pipeline will build and run this image. We can also make a script that we can run from our development environment to test our code in the same environment that the pipeline will use without having to push our code and trigger the pipeline.

@StoneyJackson
Copy link
Member

@AksharP5 Checkout test.dockerfile I dropped in the root. I think this works. Feel free to move it where you think it belongs.

So now the pipeline should build it and run it. I'll leave that to you.

@AksharP5
Copy link
Collaborator Author

@StoneyJackson the dockerfile you provided does work for the test image and it is being built in the pipeline, thanks, however the issue I am having is that when anything tries to call on plcc or plccmk, it is still failing, and it states that the commands could not be found. However the test-image should have plcc installed within it now, as it has the plcc folder in its path "root/.local/lib". So I am not sure if it is an issue with plcc being installed, which I am assuming that it isn't, or if there is something wrong with the test, but I am not sure exactly what it could be as everything except plcc seems to be functioning properly.

@StoneyJackson
Copy link
Member

@AksharP5 hmm... I had that problem when building it. But I thought I fixed it... let me see...

@StoneyJackson
Copy link
Member

@AksharP5 That worked. Now sleep on it and we'll review it Wednesday with fresh eyes.

@AksharP5
Copy link
Collaborator Author

Sounds good, thanks @StoneyJackson

@StoneyJackson StoneyJackson marked this pull request as ready for review April 3, 2024 17:29
---

Closes #12

Co-Authored-by: Akshar Patel <[email protected]>
Co-Authored-By: Stoney Jackson <[email protected]>
@StoneyJackson StoneyJackson force-pushed the akshar5/run-tests-in-pipeline-a-12 branch from 1e3538d to 7bcb74f Compare April 3, 2024 17:35
@StoneyJackson StoneyJackson merged commit ea938fc into main Apr 3, 2024
1 check passed
@StoneyJackson StoneyJackson deleted the akshar5/run-tests-in-pipeline-a-12 branch April 3, 2024 17:39
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.

2 participants