Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
# https://github.com/actions/starter-workflows/blob/master/ci/python-package.yml
# (C) Github, MIT License

# Static type checking tests using `mypy`.

name: "Python type checking"

on: [push, pull_request]
Expand Down
32 changes: 32 additions & 0 deletions .github/workflows/run-bats-tests.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# Run a set of tests, each in its own container and with a potentially customized setup.
# (Documentation and implementation for the test harness may be found in `irods/test/harness`.)

name: run-bats-tests

on: [push, pull_request]

jobs:
tests:
name: Python ${{ matrix.python }}, iRODS ${{ matrix.irods_server }}
runs-on: ubuntu-latest
defaults:
run:
working-directory: ./irods/test/harness
strategy:
matrix:
python: ['3.9','3.13']
irods_server: ['4.3.4','5.0.2']

steps:
- name: Checkout
uses: actions/checkout@v4

- name: Build images
run: ./create_docker_images.sh "${{ matrix.irods_server }}" "${{ matrix.python }}"

- name: run tests
run: |
for script in ../scripts/test[0-9]* ../login_auth_test_must_run_manually.py
do
./docker_container_driver.sh -V $script
done
29 changes: 29 additions & 0 deletions .github/workflows/run-local-suite.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# Run the client test suite in a Docker container, targeting a locally running instance of the iRODS server.
# (Documentation and implementation for the test harness may be found in `irods/test/harness`.)

name: run-local-suite

on: [push, pull_request]

jobs:
tests:
name: Python ${{ matrix.python }}, iRODS ${{ matrix.irods_server }}
runs-on: ubuntu-latest
defaults:
run:
working-directory: ./irods/test/harness
strategy:
matrix:
python: ['3.9','3.13']
irods_server: ['4.3.4','5.0.2']

steps:
- name: Checkout
uses: actions/checkout@v4

- name: Build images
run: ./create_docker_images.sh "${{ matrix.irods_server }}" "${{ matrix.python }}"

- name: run tests
run: |
./docker_container_driver.sh -V ../scripts/run_suite_locally.sh
39 changes: 39 additions & 0 deletions .github/workflows/run-the-tests.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# Create a networked set of containers (via a Docker compose project) on which to run the client test suite.
# (For further information, see the README in `docker-testing`.)

name: run-the-tests
Copy link
Member

Choose a reason for hiding this comment

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

consider a new name for this, to capture that this is a topology of containers...

perhaps... run-topology-suite or run-non-local-suite to juxtapose against run-local-suite

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here are all three and my proposed names. Please take potshots today while I address other, unrelated review comments ....

  • run-suite-under-topology
  • run-suite-under-single-node
  • run-individual-test-programs (uses single-node test harness)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

or even "run-isolated-tests" for the last one? this also conveys the fact they are run in dedicated single-mode Docker containers, and then disposed of after each test to ensure the environment is not polluted for the next test.

Copy link
Contributor

Choose a reason for hiding this comment

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

I like the names you've proposed. They provide just enough info to help the reader understand what each workflow is about.

I prefer the run-isolated-tests to run-individual-test-programs.

Is it correct to think of the names meaning this?

  • run-suite-under-topology -> run-python-suite-under-topology
  • run-suite-under-single-node -> run-python-suite-under-single-node
  • run-isolated-tests -> run-bats-tests

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, the only clarification I'd put on this list is that "run-bats-tests" is a little too narrow a description. The single-node test harness is intended to run executables in general, not necessarily BATS. We (can) currently run login_auth_test_must_run_manually.py using this harness.

Although I now see that script is not being run.

Must correct that.

Copy link
Collaborator Author

@d-w-moore d-w-moore Oct 23, 2025

Choose a reason for hiding this comment

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

Seems unrelated to the conversation?

Right, or not strictly related to the original concern of test naming. I'll just make an issue outside of this PR, where we can comment further....

Copy link
Collaborator Author

@d-w-moore d-w-moore Oct 23, 2025

Choose a reason for hiding this comment

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

Got it.

What does the run-isolated-tests workflow test? Is it currently just BATS tests or a mixture of various tests which include BATS tests?

It currently just runs the BATS tests but I'm thinking of adding the login_auth_test*.py. Which I guess would mean run-isolated-tests is a more accurate name than run-bats-tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

My only concern with that is that the word, isolated, doesn't provide enough context to the reader. I personally don't know what that means and cannot derive its meaning based on the other workflows (not that that is a requirement).

What is the distinguishing factor of the tests within the workflow?

If you include bats in the name and limit that form of tests to that workflow, it becomes very clear what it is about.

Then your login_auth_test*.py would need its own workflow file then.

If you add login_auth_test*.py to run-isolated-tests, it appears to become more of a workflow for miscellaneous test cases - i.e. there's no clear place for them, therefore they get added to the discussed workflow file.

Can you explain how one decides when a test meets the criteria for being added to run-isolated-tests?

Copy link
Collaborator Author

@d-w-moore d-w-moore Oct 24, 2025

Choose a reason for hiding this comment

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

My motivation and criterion for the single node harness design had not much to do with requiring a test to be isolated, so you're right. But ultimately, though the majority of the tests will be in BATS, it won't be all of them. BATS itself is not part of the focus of this thing.

The distinguishing thing is more that the individual test programs each need their own, potentially unique, specific setups. Allowing custom setup for each test was actually the whole point, and that's why each script can be (and often, or always will be) assigned a wrapper script. The wrapper does whatever setup is needed for the test executable, and then hands off control to that executable.

That being the primary motivator behind the single node harness thing, it also makes sense to run the login_auth_test*.py under its control. That was not a casual decision, but rather conforms well to the purpose and design of the harness. (In other words, why require a test's setup to be done manually, when you could do it automatically.)
So with that in mind, I'd suggest the name run-tests-requiring-dedicated-setup.
Awful name, which is why I preferred isolated at the beginning. But you're right , we need more specificity and accuracy in the naming.

(Context/History: In most of these tests, the so called "dedicated setup" actually setting up PAM and TLS/SSL, which is why they mostly/all have login_auth_test.sh assigned as their wrapper.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see now. Please add wording to the workflow file which explains what it's for and I think we'll be in a good spot.


on: [push, pull_request]

jobs:
tests:
name: Python ${{ matrix.python }}, iRODS ${{ matrix.irods_server }}
runs-on: ubuntu-latest
defaults:
run:
working-directory: ./docker-testing
strategy:
matrix:
python: ['3.9','3.13']
irods_server: ['4.3.4','5.0.2']

steps:
- name: Checkout
uses: actions/checkout@v4

- name: Start containers
run: ./start_containers.sh "${{ matrix.irods_server }}" "${{ matrix.python }}"

- name: run test
run: |
while :; do
client_container=$(docker ps --format "{{.Names}}"|grep python.client)
[ -n "$client_container" ] && break
sleep 1
done
echo "client_container = [$client_container]"
docker exec "${client_container}" /repo_root/docker-testing/run_tests.sh

- name: Stop containers
if: always()
run: ./stop_containers.sh "${{ matrix.irods_server }}" "${{ matrix.python }}"
1 change: 0 additions & 1 deletion Dockerfile.prc_test.centos
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,5 @@ RUN python${py_N} repo/docker_build/iinit.py \
password rods
SHELL ["/bin/bash","-c"]
CMD echo "Waiting on iRODS server... " ; \
python${PY_N} repo/docker_build/recv_oneshot -h irods-provider -p 8888 -t 360 && \
sudo groupadd -o -g $(stat -c%g /irods_shared) irods && sudo usermod -aG irods user && \
newgrp irods < repo/run_python_tests.sh
1 change: 0 additions & 1 deletion Dockerfile.prc_test.ubuntu
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,5 @@ SHELL ["/bin/bash","-c"]
# 3. run python tests as the new group

CMD echo "Waiting on iRODS server... " ; \
python${PY_N} repo/docker_build/recv_oneshot -h irods-provider -p 8888 -t 360 && \
sudo groupadd -o -g $(stat -c%g /irods_shared) irods && sudo usermod -aG irods user && \
newgrp irods < repo/run_python_tests.sh
53 changes: 53 additions & 0 deletions docker-testing/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# A Topological Setup for Testing the Python Client

The `docker-testing` directory contains the necessary files for building and
running tests from the perspective of a specific client node in a larger network.

We currently allow a choice of Python interpreter and iRODS server to be installed
on the client and provider nodes of a simulated network topology.

The choice of versions are dictated when running the test:

|:------------------:|:---------------:|
|Environment Variable| Valid Range |
|:-------------------|-----------------|
IRODS_PACKAGE_VERSION|4.3.1 to 5.0.2 |
PYTHON_VERSION |3.9 to 3.13 |
|:-------------------|-----------------|
Comment on lines +11 to +16
Copy link
Contributor

Choose a reason for hiding this comment

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

The table renders but has some extra stuff, so let's clean it up:

Suggested change
|:------------------:|:---------------:|
|Environment Variable| Valid Range |
|:-------------------|-----------------|
IRODS_PACKAGE_VERSION|4.3.1 to 5.0.2 |
PYTHON_VERSION |3.9 to 3.13 |
|:-------------------|-----------------|
| Environment Variable | Min supported version | Max supported version |
| -------------------- | --------------------- | --------------------- |
| IRODS_PACKAGE_VERSION | 4.3.1 | 5.0.2 |
| PYTHON_VERSION | 3.9 | 3.13 |


Currently the database server is fixed as Postgres.

## Details of usage

The file `$REPO/.github/workflows/run-the-tests.yml`
Copy link
Member

Choose a reason for hiding this comment

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

this file needs a better 10k-foot view starting sentence/paragraph.

what does this do? how does it fit with the other stuff that's being added?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done, pls review if desired.

(where `$REPO` is the `/path/to/local/python-irodsclient` repository)
contains commands for starting the server and client containers and running the PRC
suite in response to a push or pull-request.

The test suite can also be run on any workstation with docker compose installed.
What follows is a short summary of how to run the test configuration at the bench.
It is this procedure which is run within the Github workflows.

1. Change the working directory to the REPO top directory, e.g.:
```
cd /path/to/python-irodsclient
```

2. Run:
```
./docker-testing/start_containers.sh 4.3.4 3.11
```
This builds and runs the docker images for the project, with "4.3.4" being the iRODS
version installed on the provider and "3.11" is the version of python run on the client side.

3. Run:
```
docker exec <name-of-python-client-container> /repo_root/docker-testing/run_tests.sh
```
(Note: `/repo_root` is an actual literal path, internal to the container.)
You'll see the test output displayed on the console. At completion, xmlrunner outputs are in /tmp.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to be any more specific about where the xmlrunner output appears (i.e. a filename)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Would it be good to upload them as artifacts from the Github run?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, I'm just talking about the words written here. It says "xmlrunner outputs are in /tmp". Perhaps we can say "xmlrunner outputs can be found in /tmp in files named like /tmp/test_run_001.xml"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I guess so, but is it expected (or even guaranteed correct) practice when dealing with software which might change its own internal naming standards?


4. Tail docker logs to see the iRODS server log.
```
docker logs -f <provider_container_name>
```
1 change: 1 addition & 0 deletions docker-testing/harness-docker-compose-irods-4.yml
1 change: 1 addition & 0 deletions docker-testing/harness-docker-compose-irods-5.yml
45 changes: 45 additions & 0 deletions docker-testing/harness-docker-compose.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
version: '3'

Comment on lines +1 to +2
Copy link
Member

Choose a reason for hiding this comment

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

i don't think this is necessary/prescribed any longer - i think it can be removed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Might be nice to specify a project name up here instead with the name directive.

services:
irods-catalog:
build:
context: irods_catalog_${irods_major}
# 5432 is exposed by default and can conflict with other postgres containers.
# When the metalnx-db service is no longer needed, this stanza can be removed.
Comment on lines +7 to +8
Copy link
Member

Choose a reason for hiding this comment

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

metalnx? is this copied from irods_demo? please remove.

ports:
- "5430:5432"
Comment on lines +7 to +10
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment seems out of date and unnecessary now.

Do we need to expose the database port?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Exposing or not exposing won't be relevant to the outside environment. I could remove it though, if we think it might be dangerous.

environment:
- POSTGRES_PASSWORD=testpassword

python-client:
build:
context: python_client
args:
python_version: ${python_version}
command:
tail -f /dev/null
volumes:
- ${repo_external}:/repo_root:ro
- /tmp/irods-client-share.py-${python_version}:/irods_shared
depends_on:
irods-catalog-provider:
condition: service_healthy

irods-catalog-provider:
volumes:
- /tmp/irods-client-share.py-${python_version}:/irods_shared
build:
context: irods_catalog_provider_${irods_major}
args:
irods_version: ${irods_version}
shm_size: 500mb
healthcheck:
test: ["CMD", "su", "-", "irods", "-c", "ils || exit 1"]
interval: 10s
timeout: 10s
retries: 3
ports:
- "1247:1247"
Comment on lines +41 to +42
Copy link
Contributor

Choose a reason for hiding this comment

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

Do these ports need to be exposed?

depends_on:
- irods-catalog

48 changes: 48 additions & 0 deletions docker-testing/iinit.py
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a brief description to the top of this file explaining why it is needed.

Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
#!/usr/bin/env python

# This script creates the client environment to authenticate natively
# as the 'rods' admin on the Docker node running the Python client tests.
# Thus, we don't need irods-icommands to be installed on that node.

from irods.password_obfuscation import encode
import json
import os
import sys
from os import chmod
from os.path import expanduser,exists,join
from getopt import getopt


home_env_path = expanduser('~/.irods')
env_file_path = join(home_env_path,'irods_environment.json')
auth_file_path = join(home_env_path,'.irodsA')


def do_iinit(host, port, user, zone, password):
if not exists(home_env_path):
os.makedirs(home_env_path)
else:
raise RuntimeError('~/.irods already exists')

with open(env_file_path,'w') as env_file:
json.dump ( { "irods_host": host,
"irods_port": int(port),
"irods_user_name": user,
"irods_zone_name": zone }, env_file, indent=4)
with open(auth_file_path,'w') as auth_file:
auth_file.write(encode(password))
chmod (auth_file_path,0o600)


def get_kv_pairs_from_cmdline(*args):
arglist = list(args)
while arglist:
k = arglist.pop(0)
v = arglist.pop(0)
yield k,v


if __name__ == '__main__':
args = sys.argv[1:]
dct = {k:v for k,v in get_kv_pairs_from_cmdline(*args)}
do_iinit(**dct)
3 changes: 3 additions & 0 deletions docker-testing/irods_catalog_4/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
FROM postgres:12

COPY init-user-db.sh /docker-entrypoint-initdb.d/init-user-db.sh
11 changes: 11 additions & 0 deletions docker-testing/irods_catalog_4/init-user-db.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#!/bin/bash

# Adapted from "Initialization script" in documentation for official Postgres dockerhub:
# https://hub.docker.com/_/postgres/
set -e

psql -v ON_ERROR_STOP=1 --username "$POSTGRES_USER" --dbname "$POSTGRES_DB" <<-EOSQL
CREATE DATABASE "ICAT";
CREATE USER irods WITH PASSWORD 'testpassword';
GRANT ALL PRIVILEGES ON DATABASE "ICAT" to irods;
EOSQL
3 changes: 3 additions & 0 deletions docker-testing/irods_catalog_5/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
FROM postgres:16

COPY init-user-db.sh /docker-entrypoint-initdb.d/init-user-db.sh
12 changes: 12 additions & 0 deletions docker-testing/irods_catalog_5/init-user-db.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#!/bin/bash

# Adapted from "Initialization script" in documentation for official Postgres dockerhub:
# https://hub.docker.com/_/postgres/
set -e

psql -v ON_ERROR_STOP=1 --username "$POSTGRES_USER" --dbname "$POSTGRES_DB" <<-EOSQL
CREATE DATABASE "ICAT";
CREATE USER irods WITH PASSWORD 'testpassword';
GRANT ALL PRIVILEGES ON DATABASE "ICAT" to irods;
ALTER DATABASE "ICAT" OWNER TO irods
EOSQL
55 changes: 55 additions & 0 deletions docker-testing/irods_catalog_provider_4/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
FROM ubuntu:20.04

ENV DEBIAN_FRONTEND=noninteractive

RUN apt-get update && \
apt-get install -y \
apt-transport-https \
gnupg \
wget \
&& \
apt-get clean && \
rm -rf /var/lib/apt/lists/* /tmp/*

RUN wget -qO - https://packages.irods.org/irods-signing-key.asc | apt-key add - && \
echo "deb [arch=amd64] https://packages.irods.org/apt/ focal main" | tee /etc/apt/sources.list.d/renci-irods.list
Comment on lines +14 to +15
Copy link
Member

Choose a reason for hiding this comment

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

we have a new way to do this now - let's go ahead and use the new shiny thing


RUN apt-get update && \
apt-get install -y \
libcurl4-gnutls-dev \
jq \
python3 \
python3-distro \
python3-jsonschema \
python3-pip \
python3-psutil \
python3-requests \
rsyslog \
unixodbc \
gawk \
&& \
apt-get clean && \
rm -rf /var/lib/apt/lists/* /tmp/*

ARG irods_version=4.3.1
ARG irods_package_version_suffix=-0~focal
ARG irods_package_version=${irods_version}${irods_package_version_suffix}
ARG irods_resource_plugin_version=${irods_version}.0${irods_package_version_suffix}

RUN apt-get update && \
apt-get install -y \
irods-database-plugin-postgres=${irods_package_version} \
irods-runtime=${irods_package_version} \
irods-server=${irods_package_version} \
irods-icommands=${irods_package_version} \
&& \
apt-get clean && \
rm -rf /var/lib/apt/lists/* /tmp/*

COPY setup-${irods_version}.input /
RUN mv /setup-${irods_version}.input /irods_setup.input

WORKDIR /
COPY entrypoint.sh .
RUN chmod u+x ./entrypoint.sh
ENTRYPOINT ["./entrypoint.sh"]
Loading