Improve pipeline #677

polfeliu · 2025-12-22T09:25:33Z

Description

This pull request is a proposal to organize pipelines that depend on venvs and poetry. It has been implemented here as a previous step to demonstrate that this can be done with groovy closure patterns as a previous step to migrate it to cicdlib.

Type of change

Please add a description and delete options that are not relevant.

Create class that is able to create virtualenvironments and manage
Installer of poetry
Test Manager (extra, as I needed access to venvmanager every time)

I will also list some possible improvements that could be done. I would encourage that this be done before the migration:

Try to initialize (maybe it's not possible) the venv without the activate script. The activate script only modifies some env variables that could be set with withEnv()
Partially move the coverage stashing and merging logic to the test manager.
Move markersExcludeString or any possbile marker helper to test manager
Run all tests with the same, or variant methods of runTestHW. Maybe rename the method (INGK-1219)
Make the test more generic so it could also be migrated to cicdlib. Right now it contains references to ingenialink. I would do the assumption that test manager is for any pytest, and possibly uses the framework (not required)

Tests

Run pipeline with min, max , min_max, and all python configs

Please describe the manual tests that you ran to verify your changes if it applies.

Documentation

Please update the documentation.

Update docstrings of every function, method or class that change.
Build documentation locally to verify changes.
Add the changes at the [Unreleased] section of the CHANGELOG.

use manager for all runs use manager for all runs use manager for all runs use manager for all runs use manager for all runs use manager for all runs use manager for all runs use manager for all runs

Jenkinsfile

julieta-prieto

As mentioned in the other PR, I believe this goes beyond the scope of the original issue.

While I understand the intent behind this proposal, it feels somewhat overcomplicated for our current needs and would require significant effort to adapt across all repositories.
I’m also concerned that it may not be very intuitive for quick adoption.

Additionally, as noted in the PR description, further development in the test manager is needed before this approach can be broadly useful.

julieta-prieto · 2025-12-30T13:19:56Z

Jenkinsfile

+    *   folder: Folder path to change to (optional)
+    *   body: Closure to execute inside the folder
+    */
+    def runInFolder(folder = null, body) {


If the point of this method is to run a piece of code in a folder, why is folder optional?

julieta-prieto · 2025-12-30T13:24:38Z

Jenkinsfile

+                    activateCmd = "call ${venvName}\\Scripts\\activate\n "
+                }
+                this.runInFolder(workingDir) { folderCtx ->
+                    folderCtx.run(activateCmd + cmd)


is activateCmd + cmd concatenating commands or doing something else?

Yes, cd folder and python activation are the exceptions to the rule we are doing.
Although we can explore to avoid the activate cmd with the point I mention on the PR:

Try to initialize (maybe it's not possible) the venv without the activate script. The activate script only modifies some env variables that could be set with withEnv()

Why does it have to be an exception?
What is the difference with the rest of commands that don't need to be concatenated?

because it retains a context in the powershell and luckily the exit code is not as relevant in terms of validation. Its not that i dont want to get rid of it, its that i dont see how

julieta-prieto · 2025-12-30T13:29:01Z

Jenkinsfile

+    def wiresharkScope = ""
+    def clearSuccessfulWiresharkLogs = true
+    def startWiresharkTimeoutS = 10.0


we need to configure each of these in each pipeline, we could add that to the constructor

They are public attributes. runPythonVersions is modified for example:
https://github.com/ingeniamc/ingenialink-python/pull/677/changes#diff-e6ffa5dc854b843b3ee3c3c28f8eae2f436c2df2b1ca299cca1fa5982e390cf8R449

To be clear, there's a lot of work to be done specially on the pytestManager to make it generic enought to be migrated to cicdlib. I have focused on the venvmanager but it was easier to wrap this here locally too

I was just pointing that same as runPythonVersions, this should be there too.
If you intend to merge this PR, I would not leave the pytestManager half way. I would add everything we need to be configurable, or remove it and leave it for the future.

Jenkinsfile

julieta-prieto · 2025-12-30T13:36:25Z

Jenkinsfile

+                                            def lin_marker = markersExcludeString(HARDWARE_MARKERS + ["virtual", "no_pcap"])
+                                            venvManager.withPython(DEFAULT_PYTHON_VERSION, env.WORKING_FOLDER) { venv ->
+                                                venv.run("poetry run poe install-wheel")
+                                                venv.run("""poetry run poe tests --junitxml=pytest_reports/junit-tests-${venv.name}.xml --junit-prefix=${venv.name} -m '${lin_marker}' -o log_cli=True""")


why """ is needed here rather than single quotes?

probably not needed. I've been mininmal regarding what to change in the pipeline

What do you mean with "what to change in the pipeline"?
Isn't this line a new addition from this PR?

those lines have been changed, not fully new. but yes, we can change

julieta-prieto · 2025-12-30T13:37:17Z

Jenkinsfile

                                            sh """
                                                mkdir -p pytest_reports
-                                                cp ${LIN_DOCKER_TMP_PATH}/pytest_reports/* pytest_reports/
+                                                cp ${env.WORKING_FOLDER}/pytest_reports/* pytest_reports/
                                            """


don't we need to change this to single-line commands as well?

at some point we should

Why not now? Isn't it possible with the architecture you proposed?

julieta-prieto · 2025-12-30T13:37:41Z

Jenkinsfile

-                                                """
+                                            venvManager.forPythons(env.WORKING_FOLDER, testManager.runPythonVersions) { venv ->
+                                                venv.run("poetry run poe install-wheel")
+                                                venv.run("""poetry run poe tests --junitxml=pytest_reports/junit-tests-${venv.version}.xml --junit-prefix=${venv.version} -m virtual --setup summit_testing_framework.setups.virtual_drive.TESTS_SETUP -o log_cli=True""")


same question regarding the quotes

julieta-prieto · 2025-12-30T13:37:59Z

Jenkinsfile

                                            sh """
                                                mkdir -p pytest_reports
-                                                cp ${LIN_DOCKER_TMP_PATH}/pytest_reports/* pytest_reports/
+                                                cp ${env.WORKING_FOLDER}/pytest_reports/* pytest_reports/
                                            """


don't we need to change this to single-line commands as well?

Pol Feliu added 13 commits December 19, 2025 16:23

runInFolder method

bbd8116

add withVirtualEnv method

12d50dd

add env ctx

23bbc69

also use ctx for win

19c9f50

simplify ctx pattern

aa37acf

use ctx pattern for running in folder

3711b17

use activate script with virtualenv

2823b54

use virtualenv for any code

4b88a3d

unify createVirtualEnvironments for any platform

ee36ae9

create venv manager for creating virtual environments

6834190

use manager for all runs

40a7fb0

use manager for all runs use manager for all runs use manager for all runs use manager for all runs use manager for all runs use manager for all runs use manager for all runs use manager for all runs

use sets to manage python versions

3680c7e

simplify withVirtualenv and cleanup code

2ae394d

polfeliu requested a review from julieta-prieto December 22, 2025 13:40

polfeliu commented Dec 22, 2025

View reviewed changes

Jenkinsfile Show resolved Hide resolved

polfeliu marked this pull request as ready for review December 22, 2025 13:46

julieta-prieto reviewed Dec 29, 2025

View reviewed changes

julieta-prieto reviewed Dec 30, 2025

View reviewed changes

polfeliu mentioned this pull request Jan 7, 2026

Ingm 623 unify how tests are run ingeniamc/ingeniamotion#487

Merged

7 tasks

Improve pipeline #677

Are you sure you want to change the base?

Improve pipeline #677

Uh oh!

Conversation

polfeliu commented Dec 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

Tests

Documentation

Uh oh!

Uh oh!

julieta-prieto left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

polfeliu commented Dec 22, 2025 •

edited

Loading

julieta-prieto left a comment •

edited

Loading