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

Enables Windows and Linux build test #304

Merged
merged 3 commits into from
Nov 30, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@ on:

jobs:
test:
runs-on: ubuntu-latest
runs-on: ${{ matrix.os }}
strategy:
matrix:
python-version: ['3.8', '3.9', '3.10', 'pypy-3.8', 'pypy-3.10']
os: [ macos-latest, ubuntu-latest ]
fail-fast: false
steps:
- uses: actions/checkout@v2
Expand All @@ -24,3 +25,22 @@ jobs:
- run: pip install -r requirements.txt
- run: pip install -e .
- run: py.test

test-windows:
runs-on: windows-latest
strategy:
matrix:
python-version: ['3.9', '3.10', 'pypy-3.8', 'pypy-3.10' ]
fail-fast: false
steps:
- uses: actions/checkout@v2
- name: Set up Python
uses: actions/setup-python@v2
with:
python-version: ${{ matrix.python-version }}

- name: Create a virtual environment
run: git submodule init && git submodule update && python3 -m venv ./venv && . venv/Scripts/activate
- run: pip install -r requirements.txt
- run: pip install -e .
- run: py.test
3 changes: 2 additions & 1 deletion amazon/ionbenchmark/ion_benchmark_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ def compare_command():
with open(output_file_for_comparison, 'bw') as o:
ion.dump(report, o, binary=False)

if not args['--quiet']:
# Disabled tabulate method on Windows due to the issue#307 - https://github.com/amazon-ion/ion-python/issues/307
if not args['--quiet'] and platform.system() != 'Windows':
print(tabulate(report, tablefmt='fancy_grid', headers='keys'))

if has_regression:
Expand Down
2 changes: 1 addition & 1 deletion install.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ def _build_ionc_win():
# check_call('cmake -G \"Visual Studio 15 2017 Win64\"')
# check_call('cmake -G \"Visual Studio 16 2019\"')
check_call('cmake -G \"Visual Studio 17 2022\"')
check_call('cmake --build . --config Release')
check_call('cmake --build . --config Release --target decNumber --target ion')


def _move_lib_win(name):
Expand Down
7 changes: 0 additions & 7 deletions tests/test_benchmark_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,19 +65,12 @@ def test_option_read_no_c_extension(file=generate_test_path('integers.ion')):

def test_option_read_iterations(file=generate_test_path('integers.ion')):
# This is a potentially flaky test due to the overhead of running the CLI as a new process.
start = time.perf_counter()
(error_code, _, _) = run_cli(['read', file, '--iterations', '3'])
stop = time.perf_counter()
assert not error_code
time_1 = stop - start

start = time.perf_counter()
(error_code, _, _) = run_cli(['read', file, '--iterations', '300'])
stop = time.perf_counter()
assert not error_code
time_2 = stop - start

assert time_2 > time_1


def test_option_write_iterations(file=generate_test_path('integers.ion')):
Expand Down
10 changes: 9 additions & 1 deletion tests/test_benchmark_spec.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import platform
import sys
from os.path import abspath, join, dirname

from amazon.ionbenchmark.benchmark_spec import BenchmarkSpec
Expand All @@ -12,7 +14,13 @@ def _generate_test_path(p):


def test_get_input_file_size():
assert _minimal_spec.get_input_file_size() == 161
size = _minimal_spec.get_input_file_size()
# Different operating systems use different file structures, which, as a result, leads to varying amounts of
# space being used for storing files.
if platform.system() == 'Windows':
assert size == 167
else:
assert size == 161
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we gaining anything by asserting the apparently platform-dependent size, as opposed to, e.g., just the name of the file?

Copy link
Contributor Author

@cheqianh cheqianh Nov 29, 2023

Choose a reason for hiding this comment

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

Discussed offline, comparing the returned size against the expected size retrieved by the method Path(join(_generate_test_path("sample_spec"), _minimal_params['input_file'])).stat().st_size

where join(_generate_test_path("sample_spec"), _minimal_params['input_file']) indicates cat.ion



def test_get_format():
Expand Down
Loading