Skip to content
This repository has been archived by the owner on Apr 23, 2024. It is now read-only.

Some stupid tests #9

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Some stupid tests #9

wants to merge 2 commits into from

Conversation

recoilme
Copy link

for race detection, for example

@YuriyNasretdinov
Copy link
Contributor

Not sure what values do these tests add, especially the last one where all the contents are commented? The "race" tests don't seem to be testing that the results have actually been written anywhere as well as doing just one iteration is probably not enough.

@recoilme
Copy link
Author

Feel free to reject/close it, it's just some examples for race detection, and yes they are badly written

Copy link
Contributor

@YuriyNasretdinov YuriyNasretdinov left a comment

Choose a reason for hiding this comment

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

Please make sure you're actually checking not only lack of panics during concurrent writes but also that all results were written correctly. Otherwise this test is misleading and gives you the impression that this code works whereas even empty body for Write() func would pass this test. Please also remove all empty files / commented code ones or provide explanation on why those files are needed and what they're actually testing. Thanks

file := "table1"
const len = 4

append := func(i int) {

Choose a reason for hiding this comment

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

Suggest to use another name for func. It is not recommended to use go reserved names

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants