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

module tests #34

Closed
allonhadaya opened this issue Apr 26, 2016 · 13 comments
Closed

module tests #34

allonhadaya opened this issue Apr 26, 2016 · 13 comments
Assignees

Comments

@allonhadaya
Copy link
Contributor

I found a couple tiny bugs which I'll provide a pull request for. Basically authorship information was being set under 'author' instead of 'authors'. I wanted to write a test for it, maybe even one that checked all modules.

Would it be useful to have a set of tests that are executed against every module? A place for certain invariants to be asserted? This might help keep future modules at a high quality, especially those contributed by newcomers.

Thoughts?

@allonhadaya
Copy link
Contributor Author

#36

@fwkz
Copy link
Collaborator

fwkz commented Apr 26, 2016

This is actually very good idea. We will definitely benefit from test suit that will check all the modules in terms of compliance with RouterSploit standards. Things like if there are appropriate keys in exploits info dict etc. That way PRs with exploit modules will be checked during Travis build and contributor will get instant feedback.

@fwkz fwkz self-assigned this Apr 26, 2016
@allonhadaya
Copy link
Contributor Author

😄 awesome. I figure module tests can reuse the module loading code from interpreter.py, and somehow parametrize a test case/suite with each module. Maybe something like Python unit testing: parametrized test cases where param is the actual imported module object.

@fwkz
Copy link
Collaborator

fwkz commented Apr 26, 2016

Looks like worth digging into. I just want to point out that importing module might be time consuming. So in sake of performance we definitely have to import it just once, run all possible checks and go to the next module.

@allonhadaya
Copy link
Contributor Author

Thanks! I'm going to start a rough sketch of the idea, and keep notes on progress.

@allonhadaya
Copy link
Contributor Author

Hey @fwkz, I have a question you might be able to answer. I noticed that when interpreter looks for modules in load_modules and tries to set the current module in command_use, it's really concerned more with the Exploit class than with the module itself. Should the module tests run against the Exploit instance or the module that contains it?

@fwkz
Copy link
Collaborator

fwkz commented Apr 28, 2016

Yes, you are right. By saying module I meant actual code that does the exploitation which is obviously an Exploit class. I should be more specific. Sorry for that.

@allonhadaya
Copy link
Contributor Author

Not a problem! Thank you 😄. Let me know if you think #56 fits the bill. More tests can be added by someone who knows all the rules that exploit modules should follow.

@fwkz
Copy link
Collaborator

fwkz commented Apr 28, 2016

Good job. Everything looks great but I don't want to merge it just now because I'm thinking about factoring out the load_modules() method so it can be used in tests just like yours or in other modules (e.g. scanners). This would allow us to use modules collection in the tests without spinning the full-fledged interpreter instance.

@allonhadaya
Copy link
Contributor Author

Okay, sounds like a feature worth decoupling! I'd be happy to review that code, now that I've spent some time with this part of the system. Whenever that's available, I'll update my PR.

@fwkz
Copy link
Collaborator

fwkz commented May 3, 2016

@allonhadaya I have pushed decoupled mechanism for loading and importing the exploits. Please check out branch new_loading_mechanism. There are a few utils function:

  • utils.index_modules
  • `utils.import_exploit`
    
  • `utils.iter_modules`
    

@fwkz
Copy link
Collaborator

fwkz commented May 5, 2016

Module tests has been added. 55fa0b1

@fwkz fwkz closed this as completed May 5, 2016
@allonhadaya
Copy link
Contributor Author

@fwkz thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants