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

Move tests from nats package under test/* directory #1254

Closed

Conversation

masumomo
Copy link
Contributor

@masumomo masumomo commented Apr 25, 2023

OVERVIEW

WHAT:

  • As titled.

WHY:

  • In order to prevent nats.go from bringing nats-server dependencies to go.mod.

DETAILS OF REQUEST

Reference

Not Implemented

  • We have some tests that use internal fields, so I left them as it is for now. We will work on it in separated issues.
    • For example, nc.mu.Lock() and 'nc.mu.Unlock()' in tests.
    • I added comments // Need access to the internals for testing. for those tests.
  • TestNoRaceObjectContextOpt sometimes fails, but it's unrelated to this change.
    • As I investigated, it might come from the timing when cancelling the context and shutdown the JetStream.

go.mod Outdated
@@ -3,11 +3,18 @@ module github.com/nats-io/nats.go
go 1.19

require (
github.com/golang/protobuf v1.4.2
Copy link
Member

Choose a reason for hiding this comment

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

@masumomo Thanks for the contribution! 🙌 Could you revert the changes to go.mod and go.sum? You can instead use the following go_test.mod like this for running the tests to avoid including the server into the main go.mod:

go test ./... --count=1 -p=1 -modfile=go_test.mod

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello Wally! Oh, Thank you so much! I realised we have go_test.mod in the repo and an instruction on README to run the test 🙏 I'll revert it!

@masumomo masumomo marked this pull request as ready for review May 6, 2023 15:59
@piotrpio
Copy link
Collaborator

Hey @masumomo!

I checked out your branch, updated it and fixed remaining issues: #1441.
So I'll be closing this PR, but your contribution is merged into main (finally, sorry it took so long!).

Cheers, thanks for the help!

@piotrpio piotrpio closed this Oct 15, 2023
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.

None yet

3 participants