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

Add functional test infrastructure in CI #8

Closed
wants to merge 2 commits into from

Conversation

x41lakazam
Copy link
Contributor

Add a CI pipeline that run e2e functional test of the script generation
The functional test use the dry-run mode and compare the output with an expected output.

The test

The test receives two parameters:

  • The path to the scenario that need to be executed
  • The path to the expected output directory, this directory should be the same as the result directory written by the dry-run execution of the scenario

It executes the scenario in dry-run mode and compare the results folder with the expected output folder, if any difference is found in one of the files, the test fails.
Note that the empty lines and commented lines (the ones that start with #) are not taken into account when computing the difference.

The workflow

The workflow uses a strategy matrix to define the tests it should run, every tests run in parallel.
Note that the python setup is cached and therefore will run only if the requirements-dev.txt file is changed.

Add a new test

To add a new test:

  • Create the desired scenario
  • Create a folder under ci_tools/functional_tests/scenarios_expected_outputs/ named after the name of this test and fill it with the expected output of the scenario (you can copy the output of a valid dry run)
  • In the pipeline file (.github/workflows/functional_test.yml), add your new test in jobs.test.strategy.matrix.include (see the comment saying "Add your new test here"), follow the syntax of the other tests

@x41lakazam x41lakazam force-pushed the functional_testing_ci branch 3 times, most recently from 9fe15c1 to 8ed9095 Compare May 15, 2024 09:42
Copy link
Contributor

@amaslenn amaslenn left a comment

Choose a reason for hiding this comment

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

Please check out #6. It does nearly same thing, but using pytest:

  1. No need to run bash scripts
  2. No need to run extra pipeline, it is part of CI already.

I suggest that tests should be unified under pytest framework.

cc @TaekyungHeo @srinivas212

@TaekyungHeo
Copy link
Member

Let's wait for our PIC, @srinivas212. It seems that #6 has a cleaner structure and is well-integrated with pytest. Considering that pytest is a de facto testing framework in the PyTorch community, I believe testing with pytest is a good option. @amaslenn is an expert in pytest. I know the basic concepts of pytest, but I have not had the chance to use it in a production environment with expert use cases. Let's take this opportunity to learn and apply pytest best practices to our project.

@srinivas212
Copy link
Contributor

Thanks for your PR, @x41lakazam. Closing this as per offline discussion.

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.

4 participants