Skip to content

BEAM-13765 missing PAssert methods #16668

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.

Sign up for GitHub

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

Merged
aaltay merged 2 commits into from
Mar 8, 2022
Merged

BEAM-13765 missing PAssert methods #16668

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5ea7b3b
BEAM-13765 missing PAssert methods
mkuthan Jan 31, 2022
631f2bf
Merge branch 'apache:master' into BEAM-13765_passert_missing_methods
mkuthan Mar 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions sdks/java/core/src/main/java/org/apache/beam/sdk/testing/PAssert.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,19 @@ public interface IterableAssert<T> {
*/
IterableAssert<T> inWindow(BoundedWindow window);

/**
* Creates a new {@link IterableAssert} like this one, but with the assertion restricted to only
* run on the provided window.
*
* <p>The assertion will expect outputs to be produced to the provided window exactly once. If
* the upstream {@link Trigger} may produce output multiple times, consider instead using {@link
* #inFinalPane(BoundedWindow)} or {@link #inOnTimePane(BoundedWindow)}.
*
* @return a new {@link IterableAssert} like this one but with the assertion only applied to the
* specified window.
*/
IterableAssert<T> inOnlyPane(BoundedWindow window);
Copy link
Member

@kennknowles kennknowles Feb 1, 2022

Choose a reason for hiding this comment

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

I double checked that adequate validation is in place to ensure this is not applied when there is more than one pane. Curiously the validation occurs on the PaneInfo of emitted elements rather than by examining the trigger. But anyhow this one seems safe enough.

Copy link
Contributor Author

@mkuthan mkuthan Feb 1, 2022

Choose a reason for hiding this comment

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

What kind of validation do you mean? If I understood the code, the assertions like inOnTimePane, inLatePane, or inWindow apply a simple filter using PaneInfo to ensure that only data from given pane is passed for further assertions.

The PaneExtractors.onlyPane is somehow special because it might throw an AssertionError, it seems to be correct, the filtering is not enough to ensure that it is the only pane.

The condition for "only pane" is defined as not(!pane.isFirst() || !pane.isLast()). Wouldn't it be more straightforward to throw an error if pane.index > 0?


/**
* Creates a new {@link IterableAssert} like this one, but with the assertion restricted to only
* run on the provided window, running the checker only on the final pane for each key.
Expand Down Expand Up @@ -340,6 +353,20 @@ public interface IterableAssert<T> {

/** Builder interface for assertions applicable to a single value. */
public interface SingletonAssert<T> {
/**
* Creates a new {@link SingletonAssert} like this one, but with the assertion restricted to
* only run on the provided window.
*
* <p>The assertion will concatenate all panes present in the provided window if the {@link
Copy link
Member

@kennknowles kennknowles Feb 1, 2022

Choose a reason for hiding this comment

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

This assertion sort of applies to multiple values inherently, so that is probably why it wasn't present.

Copy link
Contributor Author

@mkuthan mkuthan Feb 1, 2022

Choose a reason for hiding this comment

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

Hmm, 'inWindow' assertion means that only window boundaries should be checked but I don't care if the element is early, on-time or late. If there is a single element in the result the SingletonAssert is needed.

For example (Scio like DSL for testing):

results should inWindow("00:00:00", "00:10:00") {
      containSingleValue("00:09:59.999", "foo") // singleton assert
}

* Trigger} produces multiple panes. If the windowing strategy accumulates fired panes and
* triggers fire multple times, consider using instead {@link #inFinalPane(BoundedWindow)} or
* {@link #inOnTimePane(BoundedWindow)}.
*
* @return a new {@link SingletonAssert} like this one but with the assertion only applied to
* the specified window.
*/
SingletonAssert<T> inWindow(BoundedWindow window);

/**
* Creates a new {@link SingletonAssert} like this one, but with the assertion restricted to
* only run on the provided window.
Expand Down Expand Up @@ -631,6 +658,11 @@ public PCollectionContentsAssert<T> inWindow(BoundedWindow window) {
return withPane(window, PaneExtractors.allPanes());
}

@Override
public PCollectionContentsAssert<T> inOnlyPane(BoundedWindow window) {
return withPane(window, PaneExtractors.onlyPane(site));
}

@Override
public PCollectionContentsAssert<T> inFinalPane(BoundedWindow window) {
return withPane(window, PaneExtractors.finalPane());
Expand Down Expand Up @@ -822,6 +854,11 @@ public PCollectionSingletonIterableAssert<T> inWindow(BoundedWindow window) {
return withPanes(window, PaneExtractors.allPanes());
}

@Override
public PCollectionSingletonIterableAssert<T> inOnlyPane(BoundedWindow window) {
return withPanes(window, PaneExtractors.onlyPane(site));
}

@Override
public PCollectionSingletonIterableAssert<T> inFinalPane(BoundedWindow window) {
return withPanes(window, PaneExtractors.finalPane());
Expand Down Expand Up @@ -943,6 +980,11 @@ private static class PCollectionSingletonAssert<T> implements SingletonAssert<T>
this.site = site;
}

@Override
public PCollectionSingletonAssert<T> inWindow(BoundedWindow window) {
return withPanes(window, PaneExtractors.allPanes());
}

@Override
public PCollectionSingletonAssert<T> inFinalPane(BoundedWindow window) {
return withPanes(window, PaneExtractors.finalPane());
Expand Down Expand Up @@ -1071,6 +1113,11 @@ private PCollectionViewAssert(
this.site = site;
}

@Override
public PCollectionViewAssert<ElemT, ViewT> inWindow(BoundedWindow window) {
return inPane(window, PaneExtractors.allPanes());
}

@Override
public PCollectionViewAssert<ElemT, ViewT> inOnlyPane(BoundedWindow window) {
return inPane(window, PaneExtractors.onlyPane(site));
Expand Down