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

Added self-hosted runner in run system tests workflow #479

Merged
merged 31 commits into from
Oct 28, 2023

Conversation

MounikaBattu17
Copy link
Contributor

@MounikaBattu17 MounikaBattu17 commented Oct 26, 2023

What does this Pull Request accomplish?

  • This PR splits run_tests workflow into run_unit_tests & run_system_tests workflows.
  • run_unit_tests:
    • runs all unit tests of ni-measurementlink-service folder ./tests/unit with and without extras install.
    • runs all tests of ni-measurementlink-generator folder.
    • runs on GitHub provided hosts (macros-latest, windows-latest, ubuntu-latest).
  • run_system_tests:
    • runs all acceptance and integration tests of ni-measurementlink-service folder ./tests/acceptance, ./tests/integration with all extras installed as these tests require real drivers.
    • used a tox environment to run these tests on multiple python version to reduce the number of runners.
    • runs on self-hosted runner (rdss-measlinkbot-win-10-py32, rdss-measlinkbot-win-10-py64).

Why should this Pull Request be merged?

Implements Task 2534163: Set up self hosted runner in repo and verify the repo pipeline
Completes Technical Debt 2487132: Need to have a self-hosted runner with MeasurementLink installed (GitHub Issue #351)

What testing has been done?

Tested pipeline run system tests workflow.

@github-actions
Copy link

github-actions bot commented Oct 26, 2023

Test Results

       40 files  +     10         40 suites  +10   45m 51s ⏱️ + 18m 26s
     479 tests ±       0       479 ✔️ +     76         0 💤  -      76  0 ±0 
16 110 runs  +2 160  14 510 ✔️ +4 440  1 600 💤  - 2 280  0 ±0 

Results for commit 86a84f3. ± Comparison against base commit f67e57b.

♻️ This comment has been updated with latest results.

@MounikaBattu17
Copy link
Contributor Author

MounikaBattu17 commented Oct 26, 2023

Addressing Brad's comment from here

I think it will be simpler to split run_test.yml into run_unit_tests.yml and run_system_tests.yml like we did in nidaqmx-python.

Other things we may want to do differently for each one:

Timeout (see nidaqmx-python)

  • Added timeout as 90 minutes

We may not want to include run_system_tests.yml in the required status checks if it's slow/unreliable

  • It is better to mark run_system_tests.yml as not required since we face some issues with py64 runner currently.

nimg tests don't need any services/drivers, so we only need to include them in run_unit_tests.yml
We may want to handle Python versions differently for run_system_tests.yml to reduce the number of runners

  • Used tox environment approach to test multiple python versions. But one of the disadvantage of this is it takes longer time to run tests than running tests on individual python versions.

We can use pytest command line options (-k or -m) to filter out tests that require drivers/services in run_unit_tests.yml so they don't add to the skip count.

  • yet to implement

Make sure the test result files have unique filenames so report_test_results.yml will treat them as separate tests.

  • I have added RUNNERNAME approach to pass runner name to tox.ini -> still in testing

@MounikaBattu17 MounikaBattu17 changed the title Added self-hosted runner in run system tests workflow [Internal] Added self-hosted runner in run system tests workflow Oct 26, 2023
@MounikaBattu17
Copy link
Contributor Author

MounikaBattu17 commented Oct 26, 2023

some observations on the runners:

  • when running jobs on individual python versions, self-hosted runner picks up the jobs faster and executes but incase on single runner requirement like running tox, it takes more time or never in picking up the job
  • py64 runner is not running even the webhook is not triggered.

@MounikaBattu17 MounikaBattu17 marked this pull request as ready for review October 26, 2023 16:52
@bkeryan bkeryan merged commit f658411 into main Oct 28, 2023
@bkeryan bkeryan deleted the users/mounika/add-self-hosted-runner branch October 28, 2023 00:40
@bkeryan bkeryan linked an issue Oct 28, 2023 that may be closed by this pull request
@vigkre vigkre changed the title [Internal] Added self-hosted runner in run system tests workflow Added self-hosted runner in run system tests workflow Oct 30, 2023
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.

Need to have a self-hosted runner with MeasurementLink installed
2 participants