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

Fixes Jenkins errors: Revert "updated with new isd format" #3888

Merged
merged 2 commits into from
Jun 2, 2020

Conversation

krlberry
Copy link
Contributor

@krlberry krlberry commented Jun 1, 2020

Reverts #3839

This is causing some failures on Jenkins (most of the test failures we are seeing across systems.) Basically, ISIS was partially updated to use the new unified ISD format ("ale") but not completely, since the new ISD format wasn't completely ready. Reverting this PR should cause these tests to start passing again:

module tests:
1326 - mex_module_test_hrsc (Failed)
1462 - newhorizons_module_test_newhorizons (Failed)

app tests:
404 - ckwriter_app_test_messenger (Failed)
906 - spkwriter_app_test_messenger (Failed)
1315 - mdisedrinfo_app_test_kernelchk (Failed)
1325 - hrsc2isis_app_test_srcImage (Failed)
1545 - voycal_app_test_linear (Failed)
1546 - voycal_app_test_nonlinear (Failed)

@krlberry
Copy link
Contributor Author

krlberry commented Jun 1, 2020

Is this going to cause failures with ALE 0.8?

I can build and test this locally. I hand-made many of these changes earlier (before Kelvin pointed out this PR) and it started working, but I didn't fully test reverting this PR.

@jessemapel
Copy link
Contributor

Is this going to cause failures with ALE 0.8?

I can build and test this locally. I hand-made many of these changes earlier (before Kelvin pointed out this PR) and it started working, but I didn't fully test reverting this PR.

I'm concerned about accidentally pulling in ALE 0.8 when creating the environment because of the >=0.7.1

@krlberry
Copy link
Contributor Author

krlberry commented Jun 1, 2020

Oh yeah, you're right, it won't work with ALE 0.8 unless I keep the changes for the header file location (and maybe there will be more failures beyond the initial cmake step.)

@krlberry krlberry marked this pull request as draft June 1, 2020 23:16
@krlberry krlberry marked this pull request as ready for review June 2, 2020 03:03
@krlberry
Copy link
Contributor Author

krlberry commented Jun 2, 2020

The previously failing tests pass locally using the changes in this PR, including using ale 0.8. Letting full tests run overnight.

@krlberry
Copy link
Contributor Author

krlberry commented Jun 2, 2020

Results from running full tests: still a few test failures (no overlaps with previous Jenkins failures) but upon examination of failing tests, they appear to require only "test data" updates, not code changes, so this PR is ready for review. (Primarily updating PVL.DIFF files and increasing tolerances for campt output that has changed due to different spice.)

For reference, these are the tests that are failing still with this PR, but I can fix by updating test data:
1215 - kaguyami2isis_app_test_level2b2 (Failed)
1326 - hrsc2isis_app_test_srcImage (Failed)
1327 - mex_module_test_hrsc (Failed)

Also, something really strange happened with my unit tests, so I didn't get a complete local unit test run. (About 25% of the unit tests failed due to comparing, for example, classA.output with classB.truth. I'm not sure how this occurred, and it's persisting this morning.)

@krlberry krlberry requested a review from jessemapel June 2, 2020 15:26
@jessemapel
Copy link
Contributor

UnitTest errors are strange. If they occur post merge on Jenkins we can look into it.

@scsides scsides merged commit 77a0a78 into dev Jun 2, 2020
@jessemapel jessemapel mentioned this pull request Jun 2, 2020
11 tasks
@jessemapel jessemapel deleted the revert-3839-isd branch June 9, 2020 21:06
jessemapel pushed a commit to jessemapel/ISIS3 that referenced this pull request Jun 11, 2020
…3888)

* Revert "updated with new isd format (DOI-USGS#3839)"

This reverts commit e926548.

* Un-revert environment.yml and include changes
jessemapel pushed a commit that referenced this pull request Jun 12, 2020
* Revert "updated with new isd format (#3839)"

This reverts commit e926548.

* Un-revert environment.yml and include changes
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