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

Fix dependencies between files #799

Merged
merged 2 commits into from
Feb 10, 2016
Merged

Fix dependencies between files #799

merged 2 commits into from
Feb 10, 2016

Conversation

tagomoris
Copy link
Member

Fix to do require for required files straightforward, per files

  • Currently, all required files are loaded by lib/fluent/load.rb even for tests
    • This file creates the implicit order of files to be loaded
    • There's no explicit dependency definition, and there are many circular dependencies
    • This situation is totally broken
  • Without this change, we can't run tests by bundle exec rake test TEST=test/file.rb
    • Running whole tests consumes long time, and it's not acceptable

This change is to make straightforward dependency from all files to others.
We can load any file in fluentd source code, make clean dependencies, and also can run each files itself to test it.

After this change, some 3rd party plugins might fail to run test code. But it's because these plugins' implicit dependencies to the other files, occasionally not loaded. It's danger situation, and should be fixed eventually.

* Currently, all required files are loaded by `lib/fluent/load.rb` even for tests
  * This file creates the implicit order of files to be loaded
  * There's no explicit dependency definition, and there are many circular dependencies
  * This situation is totally broken
* Without this change, we can't run tests by `bundle exec rake test TEST=test/file.rb`
  * Running whole tests consumes long time, and it's not acceptable

This change is to make straightforward dependency from all files to others.
We can load any file in fluentd source code, make clean dependencies, and
also can run each files itself to test it.
@tagomoris
Copy link
Member Author

FYI: I checked this change by:

  • standard test: bundle exec rake test
  • tests per each files: find test -name 'test_*.rb' | xargs -I{} -n1 bundle exec rake test TEST={}
  • execution trial: bundle exec bin/fluentd -c example/in_forward.conf

@sonots
Copy link
Member

sonots commented Feb 9, 2016

Great work

@tagomoris
Copy link
Member Author

@repeatedly I'll merge this if no objection exist.

tagomoris added a commit that referenced this pull request Feb 10, 2016
Fix dependencies between files
@tagomoris tagomoris merged commit a65118e into master Feb 10, 2016
@tagomoris
Copy link
Member Author

Merged.

@repeatedly repeatedly deleted the straight-require branch February 16, 2016 08:05
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.

2 participants