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

Fix integration tests #228

Merged
merged 1 commit into from
Mar 27, 2022
Merged

Fix integration tests #228

merged 1 commit into from
Mar 27, 2022

Conversation

javierggt
Copy link
Contributor

@javierggt javierggt commented Mar 23, 2022

Description

The following integration tests are currently failing:

FAILED Ska/engarchive/tests/test_data_source.py::test_maude_data_source - AssertionError: assert {'maude': {'f...0:00:04.900'}} == {'maude': {'f...0:00:04.900'}}
FAILED Ska/engarchive/tests/test_data_source.py::test_zero_length_fetch_maude - AssertionError: assert {'maude': {'f...nce': False}}} == {'maude': {'f...nce': False}}}
FAILED Ska/engarchive/tests/test_fetch.py::test_fetch_derived_param_aliases[sources0-DP_piTch_fss] - assert 0 > 0
FAILED Ska/engarchive/tests/test_fetch.py::test_fetch_derived_param_aliases[sources0-Calc_pitCH_fss] - assert 0 > 0
FAILED Ska/engarchive/tests/test_fetch.py::test_fetch_derived_param_aliases[sources1-DP_piTch_fss] - assert 0 > 0
FAILED Ska/engarchive/tests/test_fetch.py::test_fetch_derived_param_aliases[sources1-Calc_pitCH_fss] - assert 0 > 0
FAILED Ska/engarchive/tests/test_fetch.py::test_fetch_derived_param_aliases[sources2-DP_piTch_fss] - assert 0 > 0
FAILED Ska/engarchive/tests/test_fetch.py::test_fetch_derived_param_aliases[sources2-Calc_pitCH_fss] - assert 0 > 0

This PR fixes the first two, which come from changes in the maude package after version 3.9.0. They can also be "fixed" by downgrading maude.

The fixes in test_fetch_derived_param_aliases are still a bit of a mystery to me and do not seem to depend on maude. I did this, which is basically what the test does:

In [1]: from cheta import fetch
   ...: cxc_tstop = fetch.get_time_range('dp_pitch_fss', 'secs')[1]
   ...: dt = 2000  # seconds
   ...: d1 = fetch.Msid('dp_pitch_fss', cxc_tstop - dt, cxc_tstop + dt)
   ...: print(len(d1))
0

Then I looked into the time range:

In [2]:  fetch.get_time_range('dp_pitch_fss', 'secs')
Out[2]: (63115203.19999999, 764372455.6249999)

but if I fetch around those times, the earlies/latest times are:

(62919140.175, 764344395.2249999)

which I do not know how they relate to the time range above.

Testing

  • Passes unit tests on MacOS, linux, Windows (at least one required)
  • Functional testing

Fixes #

@taldcroft
Copy link
Member

taldcroft commented Mar 24, 2022

I don't know how I missed the failures that are being fixed here in test_maude_data_source and test_zero_length_fetch_maude since they show up now in my testing. Maybe I wasn't running all the unit tests for cheta?? Since we don't have CI in the PR process we should have the reviewer confirm that unit tests pass (see sot/skare3#797).

I cannot reproduce the test_fetch_derived_param_aliases failures however.

@jeanconn
Copy link
Contributor

You mean you want the reviewer to check that all the other ska_testr tests pass?

@taldcroft
Copy link
Member

This is a bit out of scope for the real issue here, but the get_time_range function has a bug where it does not reflect data before 2000:001. I should fix this but for the most part the start date for telemetry is sometime in 1999 and it is not that interesting. But I can't replicate the issue you highlighted with the stop time of available data:

In [18]: fetch.get_time_range('dp_pitch_fss', 'date')
Out[18]: ('2000:001:11:58:59.016', '2022:080:23:26:27.241')

In [19]: dat = fetch.MSID('dp_pitch_fss', start='2022:075')

In [20]: CxoTime(dat.times[-1]).date
Out[20]: '2022:080:23:26:27.241'

In [21]: dat = fetch.Msid('dp_pitch_fss', stop='2000:002')

In [22]: CxoTime(dat.times[0]).date
Out[22]: '1999:226:11:58:59.641'

In [23]: CxoTime(dat.times[-1]).date
Out[23]: '2000:001:13:08:32.816'

@taldcroft
Copy link
Member

You mean you want the reviewer to check that all the other ska_testr tests pass?

No, the reviewer should run the package unit tests for the PR and confirm they pass.

@taldcroft
Copy link
Member

The cheta unit tests take a little while to run, so it is common that during development I just run the ones in question. So it is credible that I forgot to do a final run of all tests. Process error so we need some new checkbox

@taldcroft
Copy link
Member

taldcroft commented Mar 24, 2022

I checked out this PR and confirmed that unit tests (the full suite) passes locally on Mac. @javierggnnt or @jeanconn what are your results with running tests locally? (with env PYTHONPATH=$HOME/git/maude where maude is at the latest master).

@javierggt
Copy link
Contributor Author

I get the same failures on my mac.

@taldcroft
Copy link
Member

taldcroft commented Mar 24, 2022

On my Mac:

In [1]: In [1]: from cheta import fetch
   ...:    ...: cxc_tstop = fetch.get_time_range('dp_pitch_fss', 'secs')[1]
   ...:    ...: dt = 2000  # seconds
   ...:    ...: d1 = fetch.Msid('dp_pitch_fss', cxc_tstop - dt, cxc_tstop + dt)
   ...:    ...: print(len(d1))
   ...: 
1952

@taldcroft
Copy link
Member

On kady (in flight ska3):

In [2]: from cheta import fetch
   ...: cxc_tstop = fetch.get_time_range('dp_pitch_fss', 'secs')[1]
   ...: dt = 2000  # seconds
   ...: d1 = fetch.Msid('dp_pitch_fss', cxc_tstop - dt, cxc_tstop + dt)
   ...: print(len(d1))
1952

@jeanconn
Copy link
Contributor

Regarding "The cheta unit tests take a little while to run, so it is common that during development I just run the ones in question. So it is credible that I forgot to do a final run of all tests. Process error so we need some new checkbox" . I'm confused. So you think that you checked the box on one or both of the recent PRs that unit tests were done here when they weren't? I thought this was an integration test failure because of the interaction with maude.

@jeanconn
Copy link
Contributor

On fido I get the "test_fetch_derived_param_aliases" failure only if I run against the ska3-flight-installed maude.

@taldcroft
Copy link
Member

I still don't understand @javierggt's test failures. I get this pass on kady in eng_archive @ master, with my maude repo at current master:

ska3-kady$ env PYTHONPATH=$HOME/git/maude pytest cheta -k aliases -v
============================= test session starts ==============================
platform linux -- Python 3.8.12, pytest-6.2.5, py-1.10.0, pluggy-1.0.0 -- /proj/sot/ska3/flight/bin/python
cachedir: .pytest_cache
hypothesis profile 'default' -> database=DirectoryBasedExampleDatabase('/data/baffin/tom/git/eng_archive/.hypothesis/examples')
rootdir: /data/baffin/tom/git/eng_archive, configfile: pytest.ini
plugins: anyio-2.2.0, hypothesis-6.29.3, arraydiff-0.3, astropy-header-0.1.2, cov-3.0.0, mock-3.6.1, doctestplus-0.11.2, openfiles-0.5.0, filter-subpackage-0.1.1, remotedata-0.3.3
collected 100 items / 94 deselected / 6 selected                               

cheta/tests/test_fetch.py::test_fetch_derived_param_aliases[sources0-DP_piTch_fss] PASSED [ 16%]
cheta/tests/test_fetch.py::test_fetch_derived_param_aliases[sources0-Calc_pitCH_fss] PASSED [ 33%]
cheta/tests/test_fetch.py::test_fetch_derived_param_aliases[sources1-DP_piTch_fss] PASSED [ 50%]
cheta/tests/test_fetch.py::test_fetch_derived_param_aliases[sources1-Calc_pitCH_fss] PASSED [ 66%]
cheta/tests/test_fetch.py::test_fetch_derived_param_aliases[sources2-DP_piTch_fss] PASSED [ 83%]
cheta/tests/test_fetch.py::test_fetch_derived_param_aliases[sources2-Calc_pitCH_fss] PASSED [100%]

@taldcroft
Copy link
Member

taldcroft commented Mar 24, 2022

On fido I get the "test_fetch_derived_param_aliases" failure only if I run against the ska3-flight-installed maude.

[EDIT] Ahh, so maybe a packaging error?! I read that wrong. Never mind.

@javierggt
Copy link
Contributor Author

I don't know what to say. I ran the test now and I do not see the test_fetch_derived_param_aliases fail.

The failure I get in test_fetch_derived_param_aliases with the older maude is different. @jeanconn Are you getting the same failure? (len(d1) == 0)

@taldcroft
Copy link
Member

So you think that you checked the box on one or both of the recent PRs that unit tests were done here when they weren't?

That is a possible process error. The tests that I was running passed but I might not have run all tests. At least that is the only way I can reconstruct seeing failures now (not the ailas ones, but the other two) which I clearly didn't see at the point of checking the box originally.

@jeanconn
Copy link
Contributor

Regarding "Are you getting the same failure? (len(d1) == 0)". I haven't seen that one.

@javierggt
Copy link
Contributor Author

javierggt commented Mar 24, 2022

Regarding "Are you getting the same failure? (len(d1) == 0)". I haven't seen that one.

Exactly. The one in the automated tests and in my local copy is that one. The error with the previous maude is a different error. I ran the same code I posted above on ska3-masters on HEAD:

In [1]: from cheta import fetch
   ...: cxc_tstop = fetch.get_time_range('dp_pitch_fss', 'secs')[1]
   ...: dt = 2000  # seconds
   ...: d1 = fetch.Msid('dp_pitch_fss', cxc_tstop - dt, cxc_tstop + dt)
   ...: print(len(d1))
1952

and now I got the same as Tom. Both times this was on HEAD, but different installations, because I have been changing things.

In my machine with 2022.3rc1:


In [1]: from cheta import fetch
   ...: with fetch.data_source('maude'):
   ...:     cxc_tstop = fetch.get_time_range('dp_pitch_fss', 'secs')[1]
   ...:     dt = 2000  # seconds
   ...:     d1 = fetch.Msid('dp_pitch_fss', cxc_tstop - dt, cxc_tstop + dt)
   ...:     print(len(d1))
   ...:
0 

@taldcroft
Copy link
Member

On fido I get the "test_fetch_derived_param_aliases" failure only if I run against the ska3-flight-installed maude.

Current master of eng_archive requires a most recent version of maude, so we should not be testing in that mode (except as a diagnostic that is useful).

@taldcroft
Copy link
Member

In my machine with 2022.3rc1:

"my machine" = your Mac? Maybe your cheta data archive is corrupted?

@javierggt
Copy link
Contributor Author

javierggt commented Mar 24, 2022

"my machine" = your Mac? Maybe your cheta data archive is corrupted?

It could be, but that is why I added the with fetch.data_source('maude'):

@taldcroft
Copy link
Member

Just looking at /export/jgonzale/ska_testr/test_outputs/logs/Linux_2022-03-23T03-01-05_48db8f5_kady.cfa.harvard.edu/Ska.engarchive there are only the two test failures that get fixed by this PR. The aliases one is not there.

=========================== short test summary info ============================
FAILED Ska/engarchive/tests/test_data_source.py::test_maude_data_source - Ass...
FAILED Ska/engarchive/tests/test_data_source.py::test_zero_length_fetch_maude
============= 2 failed, 97 passed, 1 skipped in 206.93s (0:03:26) ==============

@javierggt
Copy link
Contributor Author

javierggt commented Mar 24, 2022

How about this one?
/export/jgonzale/ska_testr/test_outputs/logs/Linux_2022-03-23T13-53-11_0adda06_kady.cfa.harvard.edu/Ska.engarchive

__________ test_fetch_derived_param_aliases[sources2-Calc_pitCH_fss] ___________

msid = 'Calc_pitCH_fss', sources = ('cxc', 'maude')

    @pytest.mark.parametrize('msid', ['DP_piTch_fss', 'Calc_pitCH_fss'])
    @pytest.mark.parametrize('sources', (('cxc',), ('maude',), ('cxc', 'maude')))
    def test_fetch_derived_param_aliases(msid, sources):
        cxc_tstop = fetch.get_time_range('dp_pitch_fss', 'secs')[1]
        with fetch.data_source(*sources):
            # Get data from 2 days to present to ensure MAUDE is queried
            dt = 200  # seconds
            d1 = fetch.Msid('piTch_fss', cxc_tstop - dt, cxc_tstop + dt)
            d2 = fetch.Msid(msid, cxc_tstop - dt, cxc_tstop + dt)
        assert d2.msid == msid  # version as the user provide
        assert d2.MSID == d1.MSID  # normalized version for accessing databases
        assert np.all(d1.times == d2.times)
        assert np.all(d1.vals == d2.vals)
>       assert len(d1) > 0
E       assert 0 > 0
E        +  where 0 = len(<Msid DP_PITCH_FSS start=2022:081:21:36:26.441 stop=2022:081:21:43:06.441 len=0 dtype=float32>)

@javierggt
Copy link
Contributor Author

I don't know why the test_fetch_derived_param_aliases tests failed yesterday, but they pass now on HEAD with 2022.3rc1. They still fail in my mac (same failure I got on HEAD yesterday), but I will see if I can figure it out.

@jeanconn
Copy link
Contributor

Maybe we can just remove all the comments here and start again? For me, the first two tests were failing and this PR fixes them, so maybe we are done?

Copy link
Member

@taldcroft taldcroft left a comment

Choose a reason for hiding this comment

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

This does indeed fix two failing integration tests and I independently confirmed that, so approving.

@taldcroft taldcroft merged commit b3fa5be into master Mar 27, 2022
@taldcroft taldcroft deleted the fix-tests branch March 27, 2022 04:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants