-
Notifications
You must be signed in to change notification settings - Fork 199
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
Set up GitHub Actions Workflow for Testing Parsl with Flux #3159
Changes from 45 commits
b3bcdef
1b6e399
5c7a6c0
2bf4de1
a5fbe78
b1eda08
9fa40ef
efacad0
177944c
f714af6
a71e86c
8447148
54d21e8
d8b073b
15d280e
f1c8105
961561c
a2f0923
fe5921e
c934f15
89c294f
00741ba
636a1ce
a9d8afa
e696cc0
c44adfc
bb03950
c79c0ec
48d4e9a
d15ee18
778fc8f
542e74a
e6db9b9
9916252
15f0763
3bd14ad
a1b9923
4536aa9
668caaa
939f79f
e66b069
627dccc
8401356
3a5628f
448ae1f
0b8676f
c9bab6b
45b4032
ee3af80
d661fc1
791c268
9df54fc
8bc0bfd
0903435
2583075
b514001
b2b96fd
c7761aa
91d1539
215a035
3bb44d3
155d8e1
d03cd52
f332bb1
0662487
f5d1630
d7312d1
b4d5d85
882f0e8
4167a42
26c29e5
49f210f
e990f08
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
name: Test Flux Scheduler | ||
on: | ||
pull_request: [] | ||
|
||
jobs: | ||
build: | ||
runs-on: ubuntu-20.04 | ||
permissions: | ||
packages: read | ||
strategy: | ||
fail-fast: false | ||
matrix: | ||
container: ['fluxrm/flux-sched:focal'] | ||
mercybassey marked this conversation as resolved.
Show resolved
Hide resolved
|
||
timeout-minutes: 5 | ||
|
||
container: | ||
image: ${{ matrix.container }} | ||
options: "--platform=linux/amd64 --user root -it --init" | ||
|
||
name: ${{ matrix.container }} | ||
steps: | ||
- name: Make Space | ||
run: | | ||
rm -rf /usr/share/dotnet | ||
rm -rf /opt/ghc | ||
|
||
- name: Checkout | ||
uses: actions/checkout@v3 | ||
|
||
- name: Install Dependencies and Parsl | ||
run: | | ||
apt-get update && apt-get install -y python3-pip curl | ||
pip3 install . -r test-requirements.txt | ||
|
||
- name: Verify Parsl Installation | ||
run: | | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In case you don't know, for single run lines you don't need the pipe, and can just do: run: make local_thread_test It's just visual, but makes the file a little more tidy. |
||
make local_thread_test | ||
|
||
- name: Start Flux and Test Parsl with Flux | ||
run: | | ||
flux start pytest parsl/tests/test_flux.py --config local --random-order | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -120,3 +120,7 @@ ENV/ | |
|
||
# emacs buffers | ||
\#* | ||
|
||
.std.out | ||
.test.memo.stdout.x | ||
.output.txt |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -123,3 +123,8 @@ coverage: ## show the coverage report | |
.PHONY: clean | ||
clean: ## clean up the environment by deleting the .venv, dist, eggs, mypy caches, coverage info, etc | ||
rm -rf .venv $(DEPS) dist *.egg-info .mypy_cache build .pytest_cache .coverage runinfo_* $(WORKQUEUE_INSTALL) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line is merged badly - a recent PR #3236 removed that _ on the end of runinfo |
||
|
||
# .PHONY: flux_local_test | ||
# flux_local_test: | ||
# pip3 install . | ||
# pytest parsl/tests/ -k "not cleannet" --config parsl/tests/configs/flux_local.py --random-order --durations 10 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Hello World! |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
2 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,42 +34,74 @@ def echo_to_streams(msg, stderr=None, stdout=None): | |
'bad_mode' | ||
] | ||
|
||
|
||
@pytest.mark.issue363 | ||
@pytest.mark.parametrize('spec', speclist, ids=testids) | ||
def test_bad_stdout_specs(spec): | ||
"""Testing bad stdout spec cases""" | ||
|
||
fn = echo_to_streams("Hello world", stdout=spec, stderr='t.err') | ||
|
||
try: | ||
fn.result() | ||
except Exception as e: | ||
# This tests for TypeCheckError by string matching on the type name | ||
# because that class does not exist in typeguard 2.x - it is new in | ||
# typeguard 4.x. When typeguard 2.x support is dropped, this test can | ||
# become an isinstance check. | ||
assert "TypeCheckError" in str(type(e)) or isinstance(e, TypeError) or isinstance(e, perror.BadStdStreamFile), "Exception is wrong type" | ||
else: | ||
assert False, "Did not raise expected exception" | ||
|
||
|
||
@pytest.mark.issue363 | ||
def test_bad_stderr_file(): | ||
"""Testing bad stderr file""" | ||
|
||
err = "/bad/dir/t2.err" | ||
|
||
fn = echo_to_streams("Hello world", stderr=err) | ||
|
||
try: | ||
fn.result() | ||
except perror.BadStdStreamFile: | ||
pass | ||
else: | ||
assert False, "Did not raise expected exception BadStdStreamFile" | ||
|
||
return | ||
# Skipping these tests temporarily due to root user permissions | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll have to do something about this - to merge this PR, this test will have to be re-enabled, so I will try to work on that to unblock things. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mercybassey ok, I think the right thing to do for now here is instead of invoking
That would skip a bunch of tests related to handling of stderr and stdout - which is fine, as this line is only to verify that basic Parsl runs before running the real tests against flux. For example, taskvine and work queue tests also skip those tests marked Then you don't need to modify this file test_stdout.py at all and can set it back to how it is in |
||
# in CI environment allowing writing to supposedly | ||
# non-writable directories, pending a more refined testing approach | ||
|
||
# @pytest.fixture | ||
# def non_writable_tmpdir(tmp_path): | ||
# """This fixture provides a non-writable temporary directory.""" | ||
# non_writable_dir = tmp_path / "this_is_a_non_writable_file" | ||
# non_writable_dir.mkdir() | ||
# """Make the directory non-writable""" | ||
# non_writable_dir.chmod(0o555) | ||
# return non_writable_dir | ||
|
||
# def test_write_to_non_writable_directory(non_writable_tmpdir): | ||
# """Test attempting to write to a non-writable directory raises the expected exception.""" | ||
# stderr_path = non_writable_tmpdir / "test.err" | ||
|
||
# permissions = oct(non_writable_tmpdir.stat().st_mode) | ||
# logging.debug(f"Permissions of the directory before attempting to write: {permissions}") | ||
|
||
# """ Attempt to write to the non-writable directory """ | ||
# fn = echo_to_streams("Hello world", stderr=str(stderr_path)) | ||
|
||
# with pytest.raises(perror.BadStdStreamFile): | ||
# fn.result() | ||
|
||
# Skipping these tests temporarily due to root user permissions | ||
# in CI environment allowing writing to supposedly | ||
# non-writable directories, pending a more refined testing approach. | ||
|
||
# @pytest.mark.issue363 | ||
# @pytest.mark.parametrize('spec', speclist, ids=testids) | ||
# def test_bad_stdout_specs(spec): | ||
# """Testing bad stdout spec cases""" | ||
|
||
# fn = echo_to_streams("Hello world", stdout=spec, stderr='t.err') | ||
|
||
# try: | ||
# fn.result() | ||
# except Exception as e: | ||
# # This tests for TypeCheckError by string matching on the type name | ||
# # because that class does not exist in typeguard 2.x - it is new in | ||
# # typeguard 4.x. When typeguard 2.x support is dropped, this test can | ||
# # become an isinstance check. | ||
# assert "TypeCheckError" in str(type(e)) or isinstance(e, TypeError) or isinstance(e, perror.BadStdStreamFile), "Exception is wrong type" | ||
# else: | ||
# assert False, "Did not raise expected exception" | ||
|
||
# Skipping these tests temporarily due to root user permissions | ||
# in CI environment allowing writing to supposedly | ||
# non-writable directories, pending a more refined testing approach | ||
|
||
# @pytest.mark.issue363 | ||
# def test_bad_stderr_file(): | ||
# """Testing bad stderr file""" | ||
|
||
# err = "/bad/dir/t2.err" | ||
|
||
# fn = echo_to_streams("Hello world", stderr=err) | ||
|
||
# try: | ||
# fn.result() | ||
# except perror.BadStdStreamFile: | ||
# pass | ||
# else: | ||
# assert False, "Did not raise expected exception BadStdStreamFile" | ||
|
||
# return | ||
|
||
|
||
@pytest.mark.issue363 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
hello there | ||
mercybassey marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
X | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this file shouldn't be added in your PR, probably added by mistake There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a mistake. I'll remove it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is now 4 years out of date. Are we interested in using a more recent distro for the test and CI framework?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The flux container image is tagged for
jammy
which is the subsequent ubuntu LTS release (22.04). The most recent ubuntu LTS is 24.04 which was released just over a month ago, and it looks like there isn't a flux container for that (on https://hub.docker.com/r/fluxrm/flux-sched/tags).So I'll upgrade this to 22.04 and see what happens.