Skip to content
This repository has been archived by the owner on Apr 7, 2022. It is now read-only.

PyTango test suite CI for Windows #369

Closed
wkitka opened this issue Jun 25, 2020 · 7 comments · Fixed by #395
Closed

PyTango test suite CI for Windows #369

wkitka opened this issue Jun 25, 2020 · 7 comments · Fixed by #395
Assignees

Comments

@wkitka
Copy link

wkitka commented Jun 25, 2020

Run PyTango test suite in AppVeyor to quickly find up Windows-related bugs.

@ajoubertza ajoubertza changed the title PyTango test suite in AppVeyor PyTango test suite CI for Windows Jun 25, 2020
@ajoubertza
Copy link
Member

It is probably easier to make a small change to the AppVeyor scripts to do this. However, GitHub Actions are another option if we find some limitation using AppVeyor.

@wkitka wkitka self-assigned this Jul 9, 2020
@wkitka wkitka assigned wkitka and unassigned wkitka Nov 2, 2020
@spirit1317 spirit1317 self-assigned this Nov 4, 2020
@spirit1317
Copy link
Contributor

First problem was that in tango/test_context.py calling:
handle, db = mkstemp()
os.unlink(db)

without previously closing the handle would result in an error.
It was solved in this commit: 7d63b8e

@spirit1317
Copy link
Contributor

spirit1317 commented Nov 4, 2020

Running more than one test using DeviceTestContext would result in Segmentation Fault occuring in the second test, as mentioned here: #387 (comment)

Two workarounds:

  1. We can first gather all test without running using --collect-only and -q options. And then use a .bat script to run all the tests one by one with all parameters separately. The solution is here: ca4f8a0
    I use a pytest_sessionfinish() hook, to generate the script, if the user called pytest with options: -collect-only -q
    Then in appveyor.yml I run generated .bat script using -c empty_file.txt option to not rely on setup.cfg pytest options.

  2. Another approach would be to use pytest-xprocess library, which can run device servers in separate process and run the tests against it. Like described here. But this is not elegant, because we have to put all tested device classes in the separate file than the tests itself. This approach was demonstrated here: abbbdb7

For now we still have to use pytango 9.3.1, because this issue is blocking: #355

@ajoubertza
Copy link
Member

@spirit1317 Thanks for the feedback. Nice work, figuring this out!

I prefer the batch file execution of the tests under Windows, ca4f8a0. I have the following comments:

  • I was wondering about the pytest_empty_config.txt file. I see we have to use that instead of setup.cfg so that it doesn't try to use the --boxed option and run all the tests in the test directory.
  • When I was trying the collect-only approach with pytest, I didn't realise that we need to specify -q twice to get the nodes IDs printed out nicely. That's good to know.
  • Capturing the test node IDs via the pytest_sessionfinish was a good idea. From a Linux background, I would have just tried to use command line tools, something like:
pytest --collect-only -q -q | grep :: | sed 's/^/pytest -c \/dev\/null -v --no-header /' > run_tests.sh
  • As you may notice above, I have included the -v and --no-header options for pytest. I think this will make the output more readable.
  • If you are going to keep the pytest_sessionfinish approach, I suggest we added a check for Windows, since the batch file only makes sense in that case. In setup.py, I see we use a check like this to detect the Windows platform.

@spirit1317
Copy link
Contributor

spirit1317 commented Dec 3, 2020

@ajoubertza Thanks for your remarks.
Like you suggested I added check for 'nt' in os.name, and repaired an old bug connected to os.unlink(). Also I found out that we need to pip install some packages for python 27. test_clinet.py was using TangoTest so I hosted this file on my repo. Maybe you will know a bettter alternative.
25012e9

Because on windows we are running each test separately, I thought it's a nice idea to add some kind of summary like here 64eba09.

Both commits include fix for windows DLL problem #355

@ajoubertza
Copy link
Member

ajoubertza commented Dec 3, 2020

Adding a summary is a good idea.

I'm not sure about the TangoTest Windows binary. Using your personal repo is not a good solution. I've asked about it on kernel dev Slack channel.

Also I found out that we need to pip install some packages for python 27

Which Python packages are missing? Or are you referring to the TangoTest executable? I don't see any additional Python packages mentioned in 25012e9.. Update: I see them now.

@spirit1317
Copy link
Contributor

I found on appveyor docs here, describing how to use appveyor REST API to send json file with a tests summary. So now we can see in appveyor tab how many tests were successful. cb9b202

I created a pull request, but I don't know if it's ok:
#395

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants