Skip to content

NIFI-10875 fixed flaky test#6718

Closed
ycliu28 wants to merge 4 commits intoapache:mainfrom
ycliu28:NIFI-10875
Closed

NIFI-10875 fixed flaky test#6718
ycliu28 wants to merge 4 commits intoapache:mainfrom
ycliu28:NIFI-10875

Conversation

@ycliu28
Copy link
Contributor

@ycliu28 ycliu28 commented Nov 24, 2022

Summary

NIFI-10875

Tracking

Please complete the following tracking steps prior to pull request creation.

Issue Tracking

Pull Request Tracking

  • Pull Request title starts with Apache NiFi Jira issue number, such as NIFI-00000
  • Pull Request commit message starts with Apache NiFi Jira issue number, as such NIFI-00000

Pull Request Formatting

  • Pull Request based on current revision of the main branch
  • Pull Request refers to a feature branch with one commit containing changes

Verification

Please indicate the verification steps performed prior to pull request creation.

Build

  • Build completed using mvn clean install -P contrib-check
    • JDK 8
    • JDK 11
    • JDK 17

Licensing

  • New dependencies are compatible with the Apache License 2.0 according to the License Policy
  • New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

  • Documentation formatting appears as expected in rendered files

@exceptionfactory
Copy link
Contributor

Thanks for the contribution @tt0suzy. Instead of changing the test to use the inline conditional, a better approach would be to assert the presence of the individual key=value parameters. That would avoid depending on parameter order while keeping the test clear.

@exceptionfactory exceptionfactory added the type: testing Pull requests for changes to test components label Nov 28, 2022
@ycliu28
Copy link
Contributor Author

ycliu28 commented Nov 28, 2022

Hi @exceptionfactory, are you suggesting something like

String querySrting = attributes.get(QuerySolr.ATTRIBUTE_SOLR_QUERY)
//"q=*:*&qt=/select&start=0&rows=10&stats=true&facet=true"
assertTrue(queryString.contains("q=*:*") && queryString.contains("qt=/select") && querystring.contains("start=0") && queryString.contains("rows=10") && queryString.contains("stats=true") && queryString.contains("facet=true"));

@exceptionfactory
Copy link
Contributor

Hi @exceptionfactory, are you suggesting something like

String querySrting = attributes.get(QuerySolr.ATTRIBUTE_SOLR_QUERY)
//"q=*:*&qt=/select&start=0&rows=10&stats=true&facet=true"
assertTrue(queryString.contains("q=*:*") && queryString.contains("qt=/select") && querystring.contains("start=0") && queryString.contains("rows=10") && queryString.contains("stats=true") && queryString.contains("facet=true"));

Thanks for the reply @tt0suzy. Yes, that is the basic idea. Instead of multiple queryString.contains() calls with in one assertTrue. Having one assert per check would be helpful to keep the code clear:

assertTrue(queryString.contains("q=*:*"));
assertTrue(queryString.contains("qt=/select"));
...

@ycliu28
Copy link
Contributor Author

ycliu28 commented Nov 28, 2022

Updated, thanks for the advice.

Copy link
Contributor

@exceptionfactory exceptionfactory left a comment

Choose a reason for hiding this comment

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

Thanks for making the changes @tt0suzy, the latest version looks good. +1 merging

asfgit pushed a commit that referenced this pull request Nov 30, 2022
- Changes avoid non-deterministic expectations for query parameters

This closes #6718

Signed-off-by: David Handermann <exceptionfactory@apache.org>
lizhizhou pushed a commit to lizhizhou/nifi that referenced this pull request Jan 2, 2023
- Changes avoid non-deterministic expectations for query parameters

This closes apache#6718

Signed-off-by: David Handermann <exceptionfactory@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: testing Pull requests for changes to test components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants