Skip to content
This repository has been archived by the owner on Jan 19, 2024. It is now read-only.

Add windows-latest to CI and fix tests on Windows #186

Closed
mikelik opened this issue May 22, 2023 · 4 comments · Fixed by #212
Closed

Add windows-latest to CI and fix tests on Windows #186

mikelik opened this issue May 22, 2023 · 4 comments · Fixed by #212
Assignees
Labels
bug Something isn't working 👍 lgtm
Milestone

Comments

@mikelik
Copy link
Contributor

mikelik commented May 22, 2023

If I run pytest tests on Windows I get 5 failing test cases:

Removing TEST_APP_DIR:  C:\Users\mikel\repo\DUNE\tests/test_app
execut cat wallet
done -execut cat wallet
================================================================= short test summary info ==================================================================
FAILED tests/test_help.py::test_help - assert b'optional arguments:' in b'usage: dune [-h] [-s NODE] [-c CONFIG_DIR] [--stop NODE] [--remove NODE]\r\n            [--list] [--simple-list] [--....
FAILED tests/test_list_features.py::test_list_features - AssertionError: assert b'execut cat ...IGNATURES\r\n' == b'GET_CODE_HA..._SIGNATURES\n'
FAILED tests/test_nodes.py::test_nodes - subprocess.CalledProcessError: Command '['C:\\Users\\mikel\\repo\\DUNE\\dune.bat', '--remove', 'done -execut cat wallet\r']' returned non-zero exit stat...
FAILED tests/test_project.py::test_create_cmake_app - AssertionError: assert ['C:\\Users\\...app.hpp', ...] == ['C:\\Users\\...app.hpp', ...]
FAILED tests/test_project.py::test_create_bare_app - AssertionError: assert ['C:\\Users\\...test_app.hpp'] == ['C:\\Users\\...test_app.hpp']
=================================================== 5 failed, 14 passed, 1 skipped in 208.45s (0:03:28) ====================================================
                                                                                                                                      
  1. Fix those tests
  2. Add Windows to CI
    Add to https://github.com/AntelopeIO/DUNE/blob/main/.github/workflows/run-tests.yml that it should be also executed on Windows (I think this might work: runs-on: [ubuntu-latest, windows-latest]

Tested on main. At least this commit has to be merged to main: dcf2dd1 to start at least some tests to pass.

@stephenpdeos
Copy link
Member

Related to #202

@mikelik mikelik self-assigned this Jun 1, 2023
@ScottBailey
Copy link
Contributor

While working another PR, I noticed quite a few path issues in the tests. Enough that I felt it was out of scope to include the changes. So I added them in this branch. For the most part, it's changing "str1" + "/str2" to os.path.join("str1","str2").

@mikelik mikelik moved this from Todo to In Progress in Team Backlog Jun 2, 2023
@mikelik
Copy link
Contributor Author

mikelik commented Jun 5, 2023

After extensive research I found it will be not possible to create dunes docker image on windows VM in github.
Issue explained: https://github.com/orgs/community/discussions/25491?sort=new#discussioncomment-3248089

I am thinking that maybe Linux could create and push Docker image and later Windows VM could run tests against it - I will verify if this is possible.

@mikelik
Copy link
Contributor Author

mikelik commented Jun 5, 2023

Unfortunately it is also not possible to run Linux containers on Windows VM in github because of the same reasons. Error is:

08f2f58: Pulling from antelopeio/dunes
image operating system "linux" cannot be used on this platform: operating system is not supported

Issue described in:
actions/runner-images#1143

In this issue I will update Windows tests so they are passing, but there will be no Windows CI 😢

@mikelik mikelik moved this from In Progress to Awaiting Review in Team Backlog Jun 5, 2023
@github-project-automation github-project-automation bot moved this from Awaiting Review to Done in Team Backlog Jun 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working 👍 lgtm
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants