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.

Sign up for GitHub

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

Jump to bottom

Removing localtest.me #886

Merged
merged 4 commits into from
Apr 13, 2021
Merged

Removing localtest.me #886

merged 4 commits into from
Apr 13, 2021

Conversation

vringar
Copy link
Contributor

@vringar vringar commented Apr 12, 2021

As it has been highly unreliable while running local tests (returning DnsNotFound errors)

Removing localtest.me
5c6a399 
As it has been highly unreliable when running
local tests (returning DnsNotFound errors)
@vringar vringar requested a review from boolean5 April 12, 2021 18:31
@codecov
Copy link

codecov bot commented Apr 12, 2021 •
edited
Loading

Codecov Report

Merging #886 (643a8b8) into master (e20fc6a) will increase coverage by 0.17%.
The diff coverage is 83.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #886      +/-   ##
==========================================
+ Coverage   50.23%   50.41%   +0.17%     
==========================================
  Files          34       34              
  Lines        3372     3372              
==========================================
+ Hits         1694     1700       +6     
+ Misses       1678     1672       -6
Impacted Files Coverage Δ
openwpm/utilities/db_utils.py 96.66% <ø> (ø)
openwpm/browser_manager.py 68.83% <80.00%> (+1.13%) ⬆️
openwpm/utilities/platform_utils.py 75.26% <100.00%> (ø)
openwpm/utilities/multiprocess_utils.py 54.76% <0.00%> (+4.76%) ⬆️

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 e20fc6a...643a8b8. Read the comment docs.

@vringar
Copy link
Contributor Author

vringar commented Apr 13, 2021

This is weird. dig is able to resolve localtest.me but both Firefox and curl are failing to do so.

boolean5
boolean5 reviewed Apr 13, 2021
View reviewed changes
test/utilities.py Outdated Show resolved Hide resolved
@vringar vringar requested a review from boolean5 April 13, 2021 14:08
boolean5
boolean5 approved these changes Apr 13, 2021
View reviewed changes
Copy link
Contributor

@boolean5 boolean5 left a comment

Choose a reason for hiding this comment

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

LGTM. I left a couple of minor comments. Feel free to merge after taking a look at them.

test/test_http_instrumentation.py Outdated Show resolved Hide resolved
test/test_simple_commands.py Show resolved Hide resolved
test/test_http_instrumentation.py Show resolved Hide resolved
test/utilities.py Show resolved Hide resolved
@vringar vringar mentioned this pull request Apr 13, 2021
Open
@vringar vringar merged commit e72ed23 into master Apr 13, 2021
@vringar vringar deleted the Remove_localtest.me branch April 13, 2021 15:39
Zaxeli pushed a commit to Zaxeli/OpenWPM that referenced this pull request Aug 10, 2021
Removing localtest.me (openwpm#886)
dadd5f4 
* Removing localtest.me

As it has been highly unreliable when running
local tests (returning DnsNotFound errors)

* Fixing tests

* Switched to localhost

* Localtest.me to localhost
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@boolean5 boolean5 boolean5 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@vringar @boolean5