OceanNetworksCanada / api-python-client

Provides easy access to ONC data in Python
https://oceannetworkscanada.github.io/api-python-client/
Apache License 2.0
10 stars 9 forks source link

Tests fail on fresh install; Test/CI instructions missing #2

Closed Jacob-Stevens-Haas closed 10 months ago

Jacob-Stevens-Haas commented 1 year ago
  1. I would like to add test instructions to README.md including how to set ${token} for the robotests. I can do it from the command line, but I need to hard code it somewhere to be able to debug (and hardcoding it in general.robot counts as a git diff). Do you also want me to setup CI for Github actions (you may need to enable it, but I can add the .yaml file that sets up the runs)?

  2. Running tests with onc installed locally results in an ImportError:

    
    jmsh@doppio:~/api-python-client$ robot .

Error in file '/home/jmsh/github/onc-python-client/tests/resources/general.robot' on line 2: Importing library '/home/jmsh/github/onc-python-client/tests/libraries/common.py' failed: ImportError: cannot import name 'ONC' from 'onc' (/home/jmsh/github/onc-python-client/onc/init.py)

I believe this is because of lines `tests/libraries/common.py` (same issue in `tests/libraries/delivery.py`):
```python
sys.path.append(os.path.join(Path(__file__).parents[2], 'onc'))
from onc import ONC

What I think common.py is trying to do: add the onc folder to sys.path, then import the onc subpackage. The problem is that appending the folder adds it to the end, meaning that any installed version of onc takes priority. It's common when working in git repos to install the current package in editable mode (pip install -e .) so it's not necessary to manipulate sys.path in testing. Solutions:

I can PR either, but I favor the latter because it respects the installed package structure. Caveat, if your internal testing workflow doesn't involve onc installed in editable mode (first test is "ONC pip package is NOT installed", but that could probably be safely removed) , this change would require changing that workflow. Could also be fixed by adding from onc import * to __init__.py, so nobody ever needs to have onc.onc in their code.

  1. The test output files (log.html, output.xml, report.html) aren't gitignored. I can PR this.

  2. 7 other failed tests:

    
    ==============================================================================
    Onc-Python-Client.Tests.Suites.Devices :: 04. Devices Test Suite              
    ==============================================================================
  3. Filter deviceName | FAIL | ('The request failed with HTTP status 400.', {'errors': [{'errorCode': 127, 'errorMessage': 'Invalid parameter value', 'parameter': 'deviceName'}]})

    Onc-Python-Client.Tests.Suites.Realtime :: 08. Real-Time Test Suite

  4. Get scalar data by location with 1 page | FAIL | density != Sensor8_Voltage

  5. Get scalar data by location with 3 pages | FAIL | density != Sensor8_Voltage

    Onc-Python-Client.Tests.Suites.Archivefiles :: 09. Archive Files Test Suite

  6. Get list by location, 1 page | FAIL | 13 != 15

  7. Get list by location, 3 pages | FAIL | 13 != 15

  8. Get list by location, 3 pages, return archiveLocations | FAIL | 4 != 15

  9. Get list by device, 3 pages, filter extension, return all meta... | FAIL | 1 != 2

    
    Are any of these expected failures?  They all seem to be working correctly, but the data on ONC's servers doesn't match what's expected in the tests (and `deviceName` seems to not be an allowed filter by the API).  I believe the way to fix them is to test for property existence (and maybe type), but not equality.  Test 4.3 is a different issue - I believe the server does not accept `deviceName` as a filter despite it being listed on the [API documentation](https://wiki.oceannetworks.ca/display/O2A/devices+Discovery+Service)
kan-fu commented 11 months ago

Hi @Jacob-Stevens-Haas, my name is Kan, and I belong to the software team of Ocean Networks Canada. It is decided within the organization that we will continue supporting this repo. Your ideas and suggestions are very valuable for improving the library, and we plan to address those issues in the new future.

Currently I will be the main maintainer for this library. Would you like to be part of the reviewer list for this repo?

Jacob-Stevens-Haas commented 11 months ago

Yes, I think I volunteered a while back and Martin Heesemann added me as a contributor, I believe? Unfortunately this has dropped off my radar for a while and I don't have the bandwidth this week to work on the repo, but hearing ONC is looking to support this module, I'll give it more attention :)