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

Should we, and is it possible, to mock the FTP for unit tests ? #249

Merged
merged 18 commits into from
Mar 10, 2023

Conversation

gmaze
Copy link
Member

@gmaze gmaze commented Mar 3, 2023

@gmaze gmaze added performance internals Internal machinery CI Continuous Integration tools labels Mar 3, 2023
@gmaze gmaze marked this pull request as draft March 3, 2023 14:27
gmaze added 14 commits March 7, 2023 09:40
- The FtpPathError is not raised anymore for non-official ftp source, a log info message is printed.
- ftpstore can now handle a specific port in the ftp source
- Replaced fsspec split_protocol by urlparse in order to access the possible port value of an ftp server
- Requires pytest-localftpserver
- In conftests set up a mocked ftp server with the content of the tutorial GDAC data
- In GDAC data fetcher tests, uses the new mocked ftp server and no longer tests the Ifremer FTP
- also improve data store
- fix bug where argo_split_path will fail in windows for a localhost ftp server
@gmaze
Copy link
Member Author

gmaze commented Mar 9, 2023

Seems to work like a charm for the FTP server !

I further gave a try to aioresponses to mock HTTP requests to the erddap, argovis, etc ...

def test_Argofetcher():
    # URI to be catched by mocked server:
    uri = 'https://erddap.ifremer.fr/erddap/tabledap/ArgoFloats.nc?data_mode,latitude,longitude,position_qc,time,time_qc,direction,platform_number,cycle_number,config_mission_number,vertical_sampling_scheme,pres,temp,psal,pres_qc,temp_qc,psal_qc,pres_adjusted,temp_adjusted,psal_adjusted,pres_adjusted_qc,temp_adjusted_qc,psal_adjusted_qc,pres_adjusted_error,temp_adjusted_error,psal_adjusted_error&platform_number=~%221901393%22&distinct()&orderBy(%22time,pres%22)'
    with aioresponses() as m:
        # Set up the expected data to return when this uri will be requested with aiohttp: 
        with open('test_data/ArgoFloats_aa81_6de2_3b5c.nc', mode='rb') as file:
            data = file.read()
        m.get(uri, body=data)

        # Move on to regular unit testing:
        ds = ArgoDataFetcher(src='erddap').float(1901393).to_xarray()
        assert isinstance(ds, xr.Dataset)

Where the test_data/ArgoFloats_aa81_6de2_3b5c.nc file is the response to the uri request.

This seems to work as well ! The ArgoDataFetcher send the http request using a fsspec httpstore but it is catched by aioresponses and the local netcdf file content.

But this method requires to have all the "expected" data stored somewhere, and I don't know where to put these.

@gmaze
Copy link
Member Author

gmaze commented Mar 10, 2023

So I implemented a mocked FTP server using https://github.com/oz123/pytest-localftpserver.
It's a fixture with a module scope and loaded by conftest.py, so it's available to any tests.

The mocked FTP is populated with all content from the tutorial gdac dataset (fetcher from the argopy-data repo)

The mocked FTP will not catch all requests to any FTP server. One has to point toward the mocked ftp server to use it.

The only modification required in tests, are on those using the ftpstore to call the GDAC.
The host argument for the ftpstore or the Data/Index fetchers is given by the global variable pytest.MOCKFTP.
(It can't be hard coded because the port number can changed from one tests session to another)

@gmaze
Copy link
Member Author

gmaze commented Mar 10, 2023

I will merge for the FTP fix to close #248
and will make a new PR to mock the erddap ...

@gmaze gmaze marked this pull request as ready for review March 10, 2023 13:56
@gmaze gmaze merged commit 0719b7e into master Mar 10, 2023
@gmaze gmaze deleted the mocked-ftp branch March 13, 2023 07:46
@gmaze gmaze mentioned this pull request Mar 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration tools internals Internal machinery performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failing CI tests because of FTP server connection issues: should we mock the FTP ?
1 participant