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

Make test should pass locally #4915

Merged
merged 14 commits into from
Sep 2, 2022
Merged

Make test should pass locally #4915

merged 14 commits into from
Sep 2, 2022

Conversation

ZackLK
Copy link
Contributor

@ZackLK ZackLK commented Jul 26, 2022

  • Fix render_test when running locally - tests can't depend on dev's local timezone.
  • Add test flags that are used to skip tests that have external dependencies.
    -- Plumb the RequiresX() functions into tests that depend on them.

Make test now runs failure-free locally.

@ZackLK ZackLK requested a review from Groxx July 26, 2022 00:17
}

// TODO: Better instructions:
// 1) How to start external dependencies - maybe link to docs so we can change it easily?
Copy link
Contributor

Choose a reason for hiding this comment

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

probably going to be something like "start cassandra (very likely docker run cassandra:version), and install the schema".
but that can come later. and there's a million acceptable ways to do it, so deciding what's best may take a few rounds.

@coveralls
Copy link

coveralls commented Jul 26, 2022

Pull Request Test Coverage Report for Build 0182fc0c-7ece-47ca-a908-0e9d0e296305

  • 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
  • 119 unchanged lines in 13 files lost coverage.
  • Overall coverage decreased (-0.6%) to 57.307%

Files with Coverage Reduction New Missed Lines %
common/util.go 2 51.46%
service/history/shard/context.go 2 67.3%
service/history/task/transfer_active_task_executor.go 2 71.58%
service/history/task/transfer_standby_task_executor.go 2 86.79%
common/persistence/nosql/nosqlplugin/cassandra/workflow.go 3 60.0%
service/matching/matcher.go 3 91.06%
common/task/fifoTaskScheduler.go 5 84.54%
service/history/execution/mutable_state_task_refresher.go 7 66.77%
common/persistence/serialization/parser.go 8 63.91%
common/persistence/serialization/thrift_decoder.go 8 57.14%
Totals Coverage Status
Change from base Build 0182fb38-3ec6-4386-81c0-7d9be8aeedff: -0.6%
Covered Lines: 85044
Relevant Lines: 148401

💛 - Coveralls

// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
// SOFTWARE.

package testflags
Copy link
Contributor

Choose a reason for hiding this comment

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

I like it better than any other name I've thought of 👍 Very clear what it's looking for / related to.

host/testcluster.go Outdated Show resolved Hide resolved
TimeField: time.Date(2000, 11, 12, 13, 14, 15, 16, time.Local),
TimeField: time.Date(2000, 11, 12, 13, 14, 15, 16, time.UTC),
Copy link
Contributor

Choose a reason for hiding this comment

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

heh. yeah, or some other fixed time zone if we want to assert that it localizes.
UTC is fine / consistent tho.

Copy link
Contributor

@Groxx Groxx left a comment

Choose a reason for hiding this comment

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

A bit of a shame that some are pushed into a base-test-helper-like-thing and some are not... but I don't think it's necessary to be consistent there. some packages are more clearly "depends on external stuff" than others.

@ZackLK ZackLK marked this pull request as ready for review August 31, 2022 22:08
@ZackLK
Copy link
Contributor Author

ZackLK commented Aug 31, 2022

I made the Makefile / buildkite changes and have ensured no tests are being skipped in buildkite, by downloading before and after test logs and comparing them. No tests are skipped in buildkite, and the exact same set of tests were run before and after my changes.

OPT_OUT_TEST=./bench/% ./canary/%
OPT_OUT_TEST=
Copy link
Contributor

Choose a reason for hiding this comment

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

bleh. I didn't notice this one before D:

@ZackLK ZackLK enabled auto-merge (squash) September 2, 2022 02:34
@ZackLK ZackLK merged commit ae1e0a0 into master Sep 2, 2022
@ZackLK ZackLK deleted the unittests branch September 2, 2022 02:56
Groxx pushed a commit that referenced this pull request Sep 6, 2022
Add test flags (controlled by environment variables) that are used to skip tests that have external
dependencies. Tests call RequiresX() functions to mark that they depend on X to run.

When tests cannot run because they require a dependency, they will be skipped and print out a message describing how to run that test (after starting your dependency.)

In order to run tests marked with a dependency, you use the following environment variables: CASSANDRA, MYSQL, MONGODB, POSTGRES. So for example, to run tests that depend on Cassandra:
CASSANDRA=1 make test
Groxx pushed a commit that referenced this pull request Oct 6, 2022
Add test flags (controlled by environment variables) that are used to skip tests that have external
dependencies. Tests call RequiresX() functions to mark that they depend on X to run.

When tests cannot run because they require a dependency, they will be skipped and print out a message describing how to run that test (after starting your dependency.)

In order to run tests marked with a dependency, you use the following environment variables: CASSANDRA, MYSQL, MONGODB, POSTGRES. So for example, to run tests that depend on Cassandra:
CASSANDRA=1 make test
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.

3 participants