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

Refactor #54

Merged
merged 17 commits into from
Dec 9, 2016
Merged

Refactor #54

merged 17 commits into from
Dec 9, 2016

Conversation

tarpas
Copy link
Owner

@tarpas tarpas commented Dec 8, 2016

This addresses issues: #53 #52 #51 #50 #32
partially #42 (poor mans solution)

With the limited test cases it worked and the performance was OK. If @blueyed @boxed tell me it works for them I'll merge and release.

@boxed
Copy link
Contributor

boxed commented Dec 8, 2016

Well it works fine with xdist as far as I can tell. Pretty cool!

@blueyed
Copy link
Contributor

blueyed commented Dec 8, 2016

Cool!

Getting an exception, likely because of an old format in .testmondata?!

Traceback (most recent call last):
  File "…/pyenv/project/bin/py.test", line 11, in <module>
    sys.exit(main())
  File "…/pyenv/project/lib/python3.5/site-packages/_pytest/config.py", line 57, in main
    return config.hook.pytest_cmdline_main(config=config)
  File "…/pyenv/project/lib/python3.5/site-packages/_pytest/vendored_packages/pluggy.py", line 745, in __call__
    return self._hookexec(self, self._nonwrappers + self._wrappers, kwargs)
  File "…/pyenv/project/lib/python3.5/site-packages/_pytest/vendored_packages/pluggy.py", line 339, in _hookexec
    return self._inner_hookexec(hook, methods, kwargs)
  File "…/pyenv/project/lib/python3.5/site-packages/_pytest/vendored_packages/pluggy.py", line 334, in <lambda>
    _MultiCall(methods, kwargs, hook.spec_opts).execute()
  File "…/pyenv/project/lib/python3.5/site-packages/_pytest/vendored_packages/pluggy.py", line 614, in execute
    res = hook_impl.function(*args)
  File "…/Vcs/pytest-testmon/testmon/pytest_testmon.py", line 82, in pytest_cmdline_main
    init_testmon_data(config)
  File "…/Vcs/pytest-testmon/testmon/pytest_testmon.py", line 76, in init_testmon_data
    affected = testmon_data.read_fs()
  File "…/Vcs/pytest-testmon/testmon/testmon_core.py", line 281, in read_fs
    self.read_data()
  File "…/Vcs/pytest-testmon/testmon/testmon_core.py", line 221, in read_data
    self.lastfailed = self._fetch_attribute('mtimes', default={}), \
  File "…/Vcs/pytest-testmon/testmon/testmon_core.py", line 178, in _fetch_attribute
    return json.loads(result[0])  # zlib.decompress(result[0]).decode('utf-8)'))
  File "/usr/lib64/python3.5/json/__init__.py", line 312, in loads
    s.__class__.__name__))
TypeError: the JSON object must be str, not 'bytes'

@tarpas
Copy link
Owner Author

tarpas commented Dec 8, 2016 via email

@blueyed
Copy link
Contributor

blueyed commented Dec 8, 2016

Yep, that worked - should be handled though automatically, shouldn't it? (or display some error that it has to be moved/deleted).

Apart from that: it seems to behave better in my small testing.
Thanks!

@boxed
Copy link
Contributor

boxed commented Dec 9, 2016

The db went from ~10 meg to ~70 meg in my case. It's fine, but something to just keep tabs on.

Looking at just the modification times of files is the problem for me still I think. It would be better for my case if it could look at the git diff since the SHA the db is from. I know it might be a bit of a special case, but maybe it's worth having some different ways to calculate changed files? Git is pretty good at this...

This change is pretty great in my testing, even for the special case I am trying to use testmon for. You should definitely merge this!

@tarpas
Copy link
Owner Author

tarpas commented Dec 9, 2016 via email

@boxed
Copy link
Contributor

boxed commented Dec 9, 2016

Ah, yea, that seems like a better solution.

@boxed
Copy link
Contributor

boxed commented Dec 9, 2016

I'm going to continue writing my tooling here like it's already doing checksums on the files and eat the extra ~3 minutes of initial startup time I get because I don't have it for now I think.

@tarpas
Copy link
Owner Author

tarpas commented Dec 9, 2016 via email

@boxed
Copy link
Contributor

boxed commented Dec 9, 2016

Great!

@tarpas
Copy link
Owner Author

tarpas commented Dec 9, 2016 via email

@boxed
Copy link
Contributor

boxed commented Dec 9, 2016

Pity. Well, in any case, will you release a new version based on this branch in the mean time?

@tarpas tarpas merged commit a40df87 into master Dec 9, 2016
@tarpas tarpas deleted the refactor branch December 9, 2016 10:28
@boxed
Copy link
Contributor

boxed commented Dec 9, 2016

The answer is yes clearly :P Thanks!

@boxed
Copy link
Contributor

boxed commented Mar 22, 2017

Btw, wanted to say it works great now for our weird use case. Thanks again!

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