Skip to content

Use XCTExpectFailure instead of skipping tests in scheme#29

Merged
mokagio merged 1 commit intotrunkfrom
add/buildkite-skip-tests-inline
Feb 7, 2022
Merged

Use XCTExpectFailure instead of skipping tests in scheme#29
mokagio merged 1 commit intotrunkfrom
add/buildkite-skip-tests-inline

Conversation

@mokagio
Copy link
Contributor

@mokagio mokagio commented Feb 4, 2022

My concern with using the scheme (or a Test Plan) to skip tests is that the setting is tucked away behind multiple UI layers and mouse clicks. I worry that we'll forget about it.

By still running these two tests and marking their failures as expected, we make the issue more visible.


PR submission checklist:

  • I have considered adding unit tests where possible. – N.A.
  • I have considered adding accessibility improvements for my changes. – N.A.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary. – N.A.

@mokagio mokagio mentioned this pull request Feb 4, 2022
@mokagio
Copy link
Contributor Author

mokagio commented Feb 4, 2022

@leandroalonso: GitHub suggested you as a reviewer (I guess because you touched the test file recently), plus @jkmassel cc-ed you in #28.

It would be great if you had the time to look at the actually fix it, making this PR redundant 😄 (In that case, please cc me because now I'm curious)

@mokagio mokagio marked this pull request as ready for review February 4, 2022 09:25
hub.loadingImage(at: 1)
hub.loadingImage(at: 0)

XCTExpectFailure("We noticed this test failing in https://github.com/wordpress-mobile/MediaEditor-iOS/pull/28 but did not have the bandwidth to fix it")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could have extracted this duplicated message in a let, but I didn't want to over-engineer what should be a temporary approach.

Base automatically changed from add/buildkite to trunk February 6, 2022 05:11
My concern with using the scheme (or a Test Plan) to skip tests is that
the setting is tucked away behind multiple UI layers and mouse clicks. I
worry that we'll forget about it.

By still running these two tests and marking their failures as expected,
we make the issue more visible.
@mokagio mokagio force-pushed the add/buildkite-skip-tests-inline branch from 0fe8320 to 094869a Compare February 7, 2022 09:42
@mokagio mokagio merged commit bc1ad84 into trunk Feb 7, 2022
@mokagio mokagio deleted the add/buildkite-skip-tests-inline branch February 7, 2022 09:52
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