Skip to content

Fallback-aware failure handling for data-bridge - integration test#35

Merged
malaskowski merged 6 commits intoKnotx:masterfrom
scypio:feature/knotx-failure-handling-tests
Dec 12, 2018
Merged

Fallback-aware failure handling for data-bridge - integration test#35
malaskowski merged 6 commits intoKnotx:masterfrom
scypio:feature/knotx-failure-handling-tests

Conversation

@scypio
Copy link
Copy Markdown
Contributor

@scypio scypio commented Dec 10, 2018

Description

Integration test for data bridge and fragment processing failure handling

Motivation and Context

New proposed feature was implemented in knotx core - Knotx/knotx#466. I would like to add an integration test to make sure that it is working

Screenshots (if appropriate)

N/A

Upgrade notes (if appropriate)

Requires Knotx/knotx#466

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

I hereby agree to the terms of the Knot.x Contributor License Agreement.

Copy link
Copy Markdown
Member

@malaskowski malaskowski left a comment

Choose a reason for hiding this comment

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

Thanks @scypio

Copy link
Copy Markdown
Member

@malaskowski malaskowski left a comment

Choose a reason for hiding this comment

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

Could you please add defaultFallback commented out with short comment (as docs) to the knotx-stack-manager/src/main/packaging/conf/application.conf ? Also please add commented out section with proper comment.

    defaultFallback = MY_GLOBAL_ID
    fallbacks = [
      {
        id = MY_GLOBAL_ID
        markup = "<knotx:fallback data-knotx-fallback-id='MY_GLOBAL_ID'><p class='error'>error</p></knotx:fallback>"
      }
    ]


@Test
@KnotxApplyConfiguration("knotx-test-app.conf")
public void whenRequestingFailingServiceWithFallback_expectFallback(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you please add one more test that checks if globally set fallback (in config) works?

Copy link
Copy Markdown
Contributor Author

@scypio scypio Dec 11, 2018

Choose a reason for hiding this comment

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

configuration example and global fallback test added

}

@Test
@KnotxApplyConfiguration("knotx-test-app-global-fallback.conf")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You may add

    defaultFallback = MY_GLOBAL_ID
    fallbacks = [
      {
        id = MY_GLOBAL_ID
        markup = "<p class='error'>global fallback</p>"
      }
    ]

directly to the knotx-test-app.conf.
If you want to add some extra config, please don't copy the whole file (if we do the change in the future we will have to change in multiple places). You may override just a part of a config, @KnotxApplyConfiguration supports this.
Look at the whenRequestingLocalSimplePageWithGetCustomAndDefaultSymbol_expectLocalSimpleHtmlWithDefault. It defines two config files:
@KnotxApplyConfiguration({"knotx-test-app.conf", "knotx-test-handlebars-custom-symbol.conf"})
Where knotx-test-handlebars-custom-symbol.conf overrides some configs from the knotx-test-app.conf.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

changed the test to use a dedicated, minimal config file

@malaskowski malaskowski merged commit 40aa141 into Knotx:master Dec 12, 2018
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