Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

SOLR-17379: Fix date parsing in Java 23, remove Lucene TestSecurityManager #3154

Merged
merged 4 commits into from
Feb 4, 2025
Merged

SOLR-17379: Fix date parsing in Java 23, remove Lucene TestSecurityManager #3154

merged 4 commits into from
Feb 4, 2025

Conversation

HoustonPutman
Copy link
Contributor

@HoustonPutman @hossman
SOLR-17379: Fix date parsing in Java 23, remove Lucene TestSecurityMa…
32f1228 
…nager

Co-authored-by: Chris Hostetter <hossman@apache.org>
@HoustonPutman
Copy link
Contributor Author

This fails every time on Crave, because a grade test runner is returning an exit code of "1". This doesn't replicate for me, but it is suspicious given that TestSecurityManager does stuff around exit codes... Trying my best to replicate locally

@HoustonPutman
Copy link
Contributor Author

Nvm, replicated it locally.

@HoustonPutman
Copy link
Contributor Author

Aha! Tracked it down to BasicAuthIntegrationTest. Wow these tests...

epugh
epugh approved these changes Feb 4, 2025
View reviewed changes
Copy link
Contributor

@epugh epugh left a comment

Choose a reason for hiding this comment

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

LGTM. Bit confused why BasicAuthIntegrationTest ever passed, but I'm sure you fix did it!

solr/core/src/test/org/apache/solr/security/BasicAuthIntegrationTest.java
ByteArrayOutputStream baos = new ByteArrayOutputStream();
PrintStream stdoutSim = new PrintStream(baos, true, StandardCharsets.UTF_8.name());
StatusTool tool = new StatusTool(stdoutSim);
try {
System.setProperty("basicauth", "harry:HarryIsUberCool");
Copy link
Contributor

Choose a reason for hiding this comment

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

any chance that this pattern shows up anywhere else and or is suggested in docs? Magic system variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have no idea, clearly it didn't work haha.

solr/core/src/test/org/apache/solr/update/processor/ParsingFieldUpdateProcessorsTest.java

assertParsedDate(
inputString,
Date.from(Instant.ofEpochMilli(expectTs)),
"parse-date-patterns-default-config");

// A bug in Java 9 (not in 8) causes this to fail! (not fixed yet?!)
Copy link
Contributor

Choose a reason for hiding this comment

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

nice clean up

@HoustonPutman
Copy link
Contributor Author

LGTM. Bit confused why BasicAuthIntegrationTest ever passed, but I'm sure you fix did it!

It worked because it just logged an error, it didn't fail the test. Pretty weird, but oh well.

@HoustonPutman HoustonPutman merged commit b779ed0 into apache:main Feb 4, 2025
3 checks passed
HoustonPutman added a commit that referenced this pull request Feb 5, 2025
@HoustonPutman @hossman
SOLR-17379: Fix date parsing in Java 23, remove Lucene TestSecurityMa…
eb07d72 
…nager (#3154)

* Fix system exit in test - by removing that part of the test

(cherry picked from commit b779ed0)

Co-authored-by: Chris Hostetter <hossman@apache.org>
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@epugh epugh epugh approved these changes

@dsmiley dsmiley dsmiley approved these changes

@hossman hossman Awaiting requested review from hossman

Assignees
No one assigned
Labels
cat:index cat:security tests tool:build
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@HoustonPutman @epugh @dsmiley