src-d / style-analyzer

Lookout Style Analyzer: fixing code formatting and typos during code reviews
GNU Affero General Public License v3.0
32 stars 21 forks source link

Optimize typos tests #668

Closed irinakhismatullina closed 5 years ago

irinakhismatullina commented 5 years ago

In accordance with #667.

irinakhismatullina commented 5 years ago

I don't understand why it can't see changed bin file in out-of-docker builds. Any ideas @zurk @vmarkovtsev?

vmarkovtsev commented 5 years ago

Still 25 minutes, does not seem to help.

irinakhismatullina commented 5 years ago

When I run all typos tests locally, it takes 3 seconds (without installs and setting up env, just tests). Can it be Travis, or too much other builds?

vmarkovtsev commented 5 years ago

@irinakhismatullina Please refer to the Travis logs, it reports the time taken by each command.

irinakhismatullina commented 5 years ago

@vmarkovtsev Well, here total for all tests (format + typos) is 920s, 15 min. It doesn't show time for smaller chunks, or I couldn't find it. Also I still don't understand why it fails out of docker here. Logs tell that the file can't be found, i just replaced it. Everything runs alright locally out of docker and inside the docker in Travis.

zurk commented 5 years ago

Add your test_ft.bin to https://github.com/src-d/style-analyzer/blob/651ee1664fb074b0dded87c3e10c6e340f5dd5f7/setup.py#L79

Should help.

irinakhismatullina commented 5 years ago

Wow, that's strange, thank you.

vmarkovtsev commented 5 years ago

@irinakhismatullina Here is what I see

screenshot 2019-03-07 at 10 37 04

Python 3.5 is doing great, however, 3.6 and 3.7 are very, very slow. You need to go through the log timings and make a report.

screenshot 2019-03-07 at 10 38 32

vmarkovtsev commented 5 years ago

Wow, that's strange, thank you.

The reason is that we test the built package, and you tested the local files. Extra files must be included into the package, otherwise, they are not installed. We had precisely the same problem in the past and that's why we actually test the package now.

zurk commented 5 years ago

Something is strange. I think it can be Travis issues. Let's restart are see if we have the same result?

UPD: restarted.

irinakhismatullina commented 5 years ago

The reason is that we test the built package, and you tested the local files. Extra files must be included into the package, otherwise, they are not installed. We had precisely the same problem in the past and that's why we actually test the package now.

Yes, I understand, it's just that I definitely replaced the filename in setup.py, but looks like that change was somehow lost in the final commit. My bad.

zurk commented 5 years ago

Ok, @vmarkovtsev it is because of https://github.com/src-d/style-analyzer/blob/master/lookout/style/format/tests/test_quality_report_noisy.py

Since we always retrain the model for a noisy report it takes a huge amount of time in a test also. I am going to fix it ASAP. (In Python 3.5 we just skip this test because of difflib)

typos test are super fast.

vmarkovtsev commented 5 years ago

OK, this means that the original PR is fine. Merging.

vmarkovtsev commented 5 years ago

@irinakhismatullina Tests failed

irinakhismatullina commented 5 years ago

I wonder, usually pandas.read_csv("*.xz") works fine, but somehow after loading from drive or reading from tempdir it became a problem, that's unexpected.