search

spacetelescope / pandokia

Regression tests framework.
Other
2 stars 4 forks source link

Adding new 'custom' field to Pandokia #32

Closed oiintam closed 5 years ago

oiintam commented 5 years ago

Since the JWST ETC client tests will test on both linux and osx systems, it is more convenience to easily separate clients tests that are tested on different operation systems and show them on Pandokia. We introduce the 'custom' field as an extra optional argument that can help further separate different tests and do proper importing and expectifying.

I have been testing on glitch, here are some example test runs:

oiintam commented 5 years ago

@cslocum can you please remind me again how I should test on okification? Thank you so much.

vglaidler commented 5 years ago

I would expect some documentation updates to go along with this change, since it is touching something a bit fundamental. I'll look through the docs & see if I can suggest likely places.

cdsontag commented 5 years ago

Wow! That touched a LOT of places. Thanks @oiintam for getting this in!

vglaidler commented 5 years ago

Needs to be vetted against use with what's running on SSB. The new field is optional & has a default value, so it shouldn't break anything - except that the DB schema is different, so there are SELECT statements that might be affected in the import..

We think that merging this won't break compatibility with ssb, but should test it via a manual import.

oiintam commented 5 years ago

I have tested a manual import with a test run pandeia_oitl_add_test_loon_2018-12-18-14:26:25 logs:

PDK_LOG.pandeia_oitl_add_test_loon_2018-12-18-14:26:25.client_linux.0
PDK_LOG.pandeia_oitl_add_test_loon_2018-12-18-14:26:25.client_osx.0
PDK_LOG.pandeia_oitl_add_test_loon_2018-12-18-14:26:25.data.0
PDK_LOG.pandeia_oitl_add_test_loon_2018-12-18-14:26:25.engine.0
PDK_LOG.pandeia_oitl_add_test_loon_2018-12-18-14:26:25.engine.1
PDK_LOG.pandeia_oitl_add_test_loon_2018-12-18-14:26:25.engine.10
PDK_LOG.pandeia_oitl_add_test_loon_2018-12-18-14:26:25.engine.11
PDK_LOG.pandeia_oitl_add_test_loon_2018-12-18-14:26:25.engine.2
PDK_LOG.pandeia_oitl_add_test_loon_2018-12-18-14:26:25.engine.3
PDK_LOG.pandeia_oitl_add_test_loon_2018-12-18-14:26:25.engine.4
PDK_LOG.pandeia_oitl_add_test_loon_2018-12-18-14:26:25.engine.5
PDK_LOG.pandeia_oitl_add_test_loon_2018-12-18-14:26:25.engine.6
PDK_LOG.pandeia_oitl_add_test_loon_2018-12-18-14:26:25.engine.7
PDK_LOG.pandeia_oitl_add_test_loon_2018-12-18-14:26:25.engine.8
PDK_LOG.pandeia_oitl_add_test_loon_2018-12-18-14:26:25.engine.9
PDK_LOG.pandeia_oitl_add_test_loon_2018-12-18-14:26:25.final.0
PDK_LOG.pandeia_oitl_add_test_loon_2018-12-18-14:26:25.final.1
PDK_LOG.pandeia_oitl_add_test_loon_2018-12-18-14:26:25.server.0

The results on Pandokia production: https://ssb.stsci.edu/pandokia/c96.cgi?query=day_report.2&test_run=pandeia_oitl_add_test_loon_2018-12-18-14%3A26%3A25 So every thing seems to be import properly. And in my log files I have client_linux and client_osx, which got reimported and is only shown as one set of client test result since there is currently no way for Pandokia production to differentiate those logs are different tests.

The same test run in glitch Pandokia for comparison: https://plglitch.stsci.edu:8443/pandokia.cgi?query=day_report.2&test_run=pandeia_oitl_add_test_loon_2018-12-18-14:26:25

cslocum commented 5 years ago

This looks great! Thanks @oiintam. I actually don't have any comments...which is strange for such a large PR. But I've been following the development before this PR.

cslocum commented 5 years ago

Hmm. Something has been broken with importing test results, which were working before. This is my latest master server run on AWS, and the tests all succeeded, but the results were not imported.

http://plglitch.stsci.edu:8080/jwst/test/reports/pandeia_master_2018-12-19-10:27:02.html

You can see the problem below at the end of this file: http://plglitch.stsci.edu:8080/jwst/test/reports/logs/pandeia_master_2018-12-19-10:27:02.server

@oiintam

cslocum commented 5 years ago

@vglaidler you took too long to review this. It has been open for a day and it is the last day of the sprint. If this can't be resolved in the next couple of hours, we need to merge it, and if you have issues with it that need to be resolved, we can file a follow-up PR.

vglaidler commented 5 years ago

If this can't be resolved in the next couple of hours, we need to merge it, and if you have issues with it that need to be resolved, we can file a follow-up PR.

That's fine. I should have elaborated that I was using the "must be approved" as "must at least be looked at in case they are bugs".

Also I do think the size of the field should be raised before merge because it's a hassle to change it afterwards.

oiintam commented 5 years ago

Hmm. Something has been broken with importing test results, which were working before. This is my latest master server run on AWS, and the tests all succeeded, but the results were not imported.

http://plglitch.stsci.edu:8080/jwst/test/reports/pandeia_master_2018-12-19-10:27:02.html

You can see the problem below at the end of this file: http://plglitch.stsci.edu:8080/jwst/test/reports/logs/pandeia_master_2018-12-19-10:27:02.server

@oiintam

Interesting, it may be a bug I introduce, I am taking a look at this. I assume the glitch Pandokia version is using the master instead of my branch, am I correct?

oiintam commented 5 years ago

@cslocum if glitch is still using the my Pandokia branch (which contain the new custom field), but the Pandeia master branch does not have the 'PDK_CUSTOM' variable yet, that may be why the import has problem. I see the in import_data.py, the default_custom variable may not be truly optional.

oiintam commented 5 years ago

I believe I have fixed the 'not really optional' default_custom variable now. I have imported the server tests and it is showing on the glitch Pandokia page.

cslocum commented 5 years ago

OK, thanks @oiintam. That makes sense. Are we ready to merge this?

oiintam commented 5 years ago

Are we ready to merge this?

deep breaths Yes 😤