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

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

HoustonPutman
Copy link
Contributor

@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...

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!

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?


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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants