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 JettyTests etopo mtime (get from test file) #213

Merged
merged 3 commits into from
Oct 9, 2024

Conversation

srstsavage
Copy link
Contributor

Description

Fixes JettyTests#testEtopoGridFiles by getting WEB-INF/ref/etopo1_ice_g_i2.bin mtime directly from file instead of hardcoding it.

Also remove gha-test from GHA Maven build/target branches (no longer needed).

Note: trying a fresh PR because GHA test failed and I don't have permissions to rerun them.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist before requesting a review

  • I have performed a self-review of my code
  • My code follows the style guidelines of this project
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Also remove `gha-test` from GHA Maven build/target branches
@ChrisJohnNOAA
Copy link
Contributor

It looks like the testGpcp failed, but that is due to an external server that recently stopped responding. I'm not sure if there's an easy change there or if we should disable that test.

@srstsavage
Copy link
Contributor Author

Yeah, looks like FileVisitorDNLSTests#testGpcp was the only failure due to this page not returning expected results:

https://www.ncei.noaa.gov/data/global-precipitation-climatology-project-gpcp-daily/

Many NCEI systems are currently down due to impacts from Hurricane Helene

https://www.noaa.gov/news/some-noaa-ncei-websites-systems-down-due-to-helene-devastation-in-asheville-nc

and I don't believe there's an expected timeline for getting them back online yet. So I agree, probably makes sense to disable/skip that test for now, and make a note or issue to restore it once the service is available again.

@srstsavage
Copy link
Contributor Author

Added that temporary test skip and a context init fix (context.setDatasetsThatFailedToLoadSB(new StringBuilder())) that you previously identified. Should be good assuming the tests pass this time.

Copy link
Contributor

@ChrisJohnNOAA ChrisJohnNOAA left a comment

Choose a reason for hiding this comment

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

Looks good, but I'll wait for the tests to finish before merging.

@ChrisJohnNOAA ChrisJohnNOAA merged commit 5076896 into ERDDAP:main Oct 9, 2024
1 check failed
@ChrisJohnNOAA
Copy link
Contributor

The failure on the action is likely a permission problem. The tests all passed, I merged this pull.

@srstsavage
Copy link
Contributor Author

Build failed with a single test failure

Error:  Errors: 
Error:    JettyTests.checkForUncaughtSpecialText:4840 Runtime 
* Trouble: java.io.IOException: HTTP status code=503 for URL: http://localhost:8080/erddap/de/tabledap/glerlAvgTemp.graph
(Error {
    code=503;
    message="Service Unavailable: Es gab ein (vorübergehendes?) Problem. Warten Sie eine Minute und versuchen Sie es dann erneut. (Klicken Sie in einem Browser auf die Schaltfläche Neu laden.)";
})

which seems intermittent. I don't have permission to re-run.

@srstsavage
Copy link
Contributor Author

Also for the automated dependency tracking/submission, here's the plugin page:

https://github.com/marketplace/actions/maven-dependency-tree-dependency-submission

If you'd prefer to wait on setting up dependency graph tracking we can remove this bit for now.

@ChrisJohnNOAA
Copy link
Contributor

Also for the automated dependency tracking/submission, here's the plugin page:

https://github.com/marketplace/actions/maven-dependency-tree-dependency-submission

If you'd prefer to wait on setting up dependency graph tracking we can remove this bit for now.

I think everything is set up, but it was still failing. I removed it for now.

Relatedly while I was looking through settings as part of this, I enabled this as a separate workflow:
https://github.com/ERDDAP/erddap/actions/workflows/dependency-graph/auto-submission
which I believe includes the maven dependency submission, so maybe we're all set?

@srstsavage
Copy link
Contributor Author

Yep, seems to be working!

https://github.com/ERDDAP/erddap/network/dependencies

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.

2 participants