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

Fix config #561

Closed
wants to merge 16 commits into from
Closed

Fix config #561

wants to merge 16 commits into from

Conversation

b1ron
Copy link
Contributor

@b1ron b1ron commented Aug 21, 2023

Refs #556, #557.

@b1ron b1ron added the code/chore Dance tool code maintenance improvements label Aug 21, 2023
@b1ron b1ron self-assigned this Aug 21, 2023
@b1ron b1ron marked this pull request as ready for review August 21, 2023 14:45
@b1ron b1ron requested a review from a team as a code owner August 21, 2023 14:45
@b1ron b1ron requested a review from AlekSi August 21, 2023 14:45
@b1ron b1ron enabled auto-merge (squash) August 21, 2023 14:45
@@ -87,8 +87,8 @@ results:
ferretdb:
stats:
expected_fail: 10
expected_skip: 19
expected_pass: 633
Copy link
Member

Choose a reason for hiding this comment

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

Why the number of passing tests went down?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's needs some investigation.

Copy link
Contributor Author

@b1ron b1ron Aug 28, 2023

Choose a reason for hiding this comment

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

We seem to be running less subtests in total with the update. This is not the case for MongoDB. I am unsure why this is happening. I took a lot at the changelog but could not find a commit that might be the culprit. I logged some more output to verify that:

FerretDB:

CURRENT
commit: 6d814184973ae7ffc13281c93b423dc522a4bcca
Expectedly ignored: 16
Total number of tests: 678

UPDATE
commit: 6c7e1241b747d2fb5d164c9ca9d17617f33e7899
Expectedly ignored: 15
Total number of tests: 662

Any ideas?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This bumped our passes back up a little.

@@ -157,6 +155,8 @@ results:
- go.mongodb.org/mongo-driver/x/mongo/driver/integration/TestAggregate/Multiple_Batches
- go.mongodb.org/mongo-driver/x/mongo/driver/integration/TestAggregate/TestMaxTimeMSInGetMore
fail:
- go.mongodb.org/mongo-driver/mongo/integration/TestCausalConsistency_NotSupported
Copy link
Member

Choose a reason for hiding this comment

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

There must be a comment explaining why those tests fail

Copy link
Contributor Author

@b1ron b1ron Aug 25, 2023

Choose a reason for hiding this comment

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

I need to look into it more. They were previously passing now they panic.

	=== RUN   TestCausalConsistency_NotSupported
	--- FAIL: TestCausalConsistency_NotSupported (0.08s)
	
go.mongodb.org/mongo-driver/mongo/integration/TestCausalConsistency_NotSupported/afterClusterTime_not_included:
	=== RUN   TestCausalConsistency_NotSupported/afterClusterTime_not_included
	    --- FAIL: TestCausalConsistency_NotSupported/afterClusterTime_not_included (0.08s)
	panic: runtime error: invalid memory address or nil pointer dereference [recovered]
		panic: runtime error: invalid memory address or nil pointer dereference
	[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0xe47b76]
	
	goroutine 72 [running]:
	testing.tRunner.func1.2({0xecf8e0, 0x17877e0})
		/usr/local/go/src/testing/testing.go:1526 +0x372
	testing.tRunner.func1()
		/usr/local/go/src/testing/testing.go:1529 +0x650
	panic({0xecf8e0, 0x17877e0})
		/usr/local/go/src/runtime/panic.go:890 +0x263
	go.mongodb.org/mongo-driver/mongo/integration.TestCausalConsistency_NotSupported.func1(0xc0002f0c40)
		/home/byron/workdir/dance/tests/mongo-go-driver/mongo/integration/causal_consistency_test.go:225 +0x256
	go.mongodb.org/mongo-driver/mongo/integration/mtest.(*T).RunOpts.func1(0x0?)
		/home/byron/workdir/dance/tests/mongo-go-driver/mongo/integration/mtest/mongotest.go:266 +0x5a9
	testing.tRunner(0xc0002fbba0, 0xc00003cde0)
		/usr/local/go/src/testing/testing.go:1576 +0x217
	created by testing.(*T).Run
		/usr/local/go/src/testing/testing.go:1629 +0x806

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 am not super confident on this but it seems that the changes made now cause their event monitor to return nil here when it tries to inspect the command name. Could be related to mongodb/mongo-go-driver#1347?

@AlekSi should I add this failing test to common ignore for now?

Copy link
Contributor Author

@b1ron b1ron Aug 28, 2023

Choose a reason for hiding this comment

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

But it only fails on FerretDB which invalidates the above. 👁️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AlekSi should I add this failing test to common ignore for now?

Added to FerretDB failures.

tests/mongo.yml Outdated Show resolved Hide resolved
@b1ron b1ron requested a review from AlekSi August 28, 2023 16:08
@AlekSi AlekSi marked this pull request as draft September 27, 2023 11:39
auto-merge was automatically disabled September 27, 2023 11:39

Pull request was converted to draft

@mergify
Copy link
Contributor

mergify bot commented Sep 27, 2023

@b1ron this pull request has merge conflicts.

@mergify mergify bot added the conflict PRs that have merge conflicts label Sep 27, 2023
@b1ron b1ron closed this Oct 3, 2023
@mergify mergify bot removed the conflict PRs that have merge conflicts label Oct 3, 2023
@b1ron b1ron deleted the fix-config branch December 5, 2023 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code/chore Dance tool code maintenance improvements
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants