marco-c / autowebcompat

Automatically detect web compatibility issues
Mozilla Public License 2.0
34 stars 41 forks source link

Port Crawler Improvements #258

Open Shashi456 opened 6 years ago

Shashi456 commented 6 years ago

Fixes #236

codecov-io commented 6 years ago

Codecov Report

Merging #258 into master will decrease coverage by 0.19%. The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #258     +/-   ##
=========================================
- Coverage   15.83%   15.64%   -0.2%     
=========================================
  Files          13       13             
  Lines        1894     1918     +24     
  Branches      328      331      +3     
=========================================
  Hits          300      300             
- Misses       1592     1616     +24     
  Partials        2        2
Impacted Files Coverage Δ
collect.py 0% <0%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c7773a9...c13f5a8. Read the comment docs.

Shashi456 commented 6 years ago

@sagarvijaygupta @marco-c so i made already_clicked_elems a local variable and took care of indentation, there are redundancies in the exception blocks because we check if the xpath exists or not, so there's a try-catch block for each conditional.

marco-c commented 6 years ago

there are redundancies in the exception blocks because we check if the xpath exists or not, so there's a try-catch block for each conditional.

What if you put the try before the if-else for xpath?

marco-c commented 6 years ago

so i made already_clicked_elems a local variable

It can't be local to do_something, otherwise it gets cleared every time we click on something and thus it becomes ineffective.

Shashi456 commented 6 years ago

@marco-c where should i declare it instead? because one way i can think of is declaring it in the function which is calling it and pass it as a parameter

sagarvijaygupta commented 6 years ago

@Shashi456 I think we should declare it in run_test_both and pass it to jump_back also.

marco-c commented 6 years ago

@Shashi456 I think we should declare it in run_test_both and pass it to jump_back also.

Yes, we should declare it in run_test_both. Do we need it for jump_back? In jump_back we should already know exactly what we're going to click, right?