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

common: treat code compiled with coverage as debug #570

Closed
wants to merge 1 commit into from

Conversation

fruch
Copy link
Contributor

@fruch fruch commented Mar 27, 2024

since there are some slowdowns with coverage enabled binary we temporary treat it as debug, so all test that were tuned down for debug in dtest, would be have the same as they do for debug.

since there are some slowdowns with coverage enabled binary
we temporary treat it as debug, so all test that were
tuned down for debug in dtest, would be have the same
as they do for debug.
@fruch
Copy link
Contributor Author

fruch commented Mar 27, 2024

@benipeled

feel free to take it for a ride in jenkins

I've test this locally run test_bulk_round_trip_with_timeouts x30 in parralell

pytest --scylla-version=cov --log-cli-level=debug -n 3 --count=30 cqlsh_tests/cqlsh_copy_tests.py::TestCqlshCopy::test_bulk_round_trip_with_timeouts 

before this change 80% of the runs would fail, after it all of them passed

@roydahan
Copy link
Contributor

LGTM, but now I'm rethinking if we need to run in CI both this and the regular dtest.
My main concern is that something pass CI like this and then will fail in next for the regular run.

@fruch
Copy link
Contributor Author

fruch commented Mar 27, 2024

LGTM, but now I'm rethinking if we need to run in CI both this and the regular dtest. My main concern is that something pass CI like this and then will fail in next for the regular run.

The assumption the coverage doesn't slow down anything, is probably wrong, and you'll still have this issue even if we'll move this part into dtest and do that for specific places

@roydahan
Copy link
Contributor

The assumption the coverage doesn't slow down anything, is probably wrong, and you'll still have this issue even if we'll move this part into dtest and do that for specific places

There was not such assumption, the assumption is that the tests should handle or be able to run on any machine, slow or fast.
We know it's not the case for all existing tests, but once we introduce this feature in CI, it'll start "enforcing" it for people.

@fruch
Copy link
Contributor Author

fruch commented Apr 4, 2024

@fruch seems like it works for test_bulk_round_trip_with_timeouts [0] but raised a few other failures https://jenkins.scylladb.com/job/scylla-master/job/byo/job/byo_build_tests_dtest/2192

https://jenkins.scylladb.com/job/scylla-master/job/byo/job/byo_build_tests_dtest/2192/testReport/cqlsh_tests.cqlsh_copy_tests/TestCqlshCopy/Tests___dtest___test_bulk_round_trip_with_timeouts/

  • paxos and lwt tests are failing cause of the error injection api doesn't work.
  • it proved some test now pass
  • there are more issue, which are not fixed by this, and would be need to handle/looked at

so the direction should be not to change the mode, but to this this code/logic into dtest, and do it for the specific tests
it would help, and then continue to review the rest of the issues

@fruch
Copy link
Contributor Author

fruch commented May 1, 2024

@benipeled move this code into dtest, closing

@fruch fruch closed this May 1, 2024
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