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-16676: Test improvements #1443

Closed
wants to merge 2 commits into from

Conversation

risdenk
Copy link
Contributor

@risdenk risdenk commented Mar 8, 2023

@risdenk risdenk self-assigned this Mar 8, 2023
@risdenk risdenk force-pushed the SOLR-16676-test-improvements branch from e2348d0 to 6387cf7 Compare March 8, 2023 15:26
@stillalex
Copy link
Member

sorry, there are a lot of cosmetic changes here. while I don't disagree with these, they are 'nice to have' rather than adding some meaningful change to understand why this might be failing. if you don't mind, could you focus on failure-fixing patches first to get the builds passing reliably, and then we can make the code clean(er) and nice(er).

@Override
public void onSuccess(NamedList<Object> t) {
assertTrue(value, value.equals(MDC.get(key)));
assertEquals(value, MDC.get(key));
Copy link
Member

Choose a reason for hiding this comment

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

this is a good catch, much better to have the values included in the assertion message.

@risdenk
Copy link
Contributor Author

risdenk commented Mar 8, 2023

sorry, there are a lot of cosmetic changes here. while I don't disagree with these, they are 'nice to have' rather than adding some meaningful change to understand why this might be failing. if you don't mind, could you focus on failure-fixing patches first to get the builds passing reliably, and then we can make the code clean(er) and nice(er).

I'm with you I just put this together to try to see if there were missing assertions and general cleanup. I agree it would be better to fix the underlying issue first.

@risdenk
Copy link
Contributor Author

risdenk commented Mar 9, 2023

combined into #1444

@risdenk risdenk closed this Mar 9, 2023
@risdenk risdenk deleted the SOLR-16676-test-improvements branch March 9, 2023 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants