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

test for getFileInfo #544

Closed
wants to merge 14 commits into from
Closed

test for getFileInfo #544

wants to merge 14 commits into from

Conversation

uniyalprashant9
Copy link
Contributor

@uniyalprashant9 uniyalprashant9 commented Jun 18, 2019

#424
Tests for getFileInfo

@uniyalprashant9
Copy link
Contributor Author

uniyalprashant9 commented Jun 18, 2019

It needs an absolute path instead of relative path.

@codeclimate
Copy link

codeclimate bot commented Jun 18, 2019

Code Climate has analyzed commit 61fdc08 and detected 0 issues on this pull request.

View more on Code Climate.

@uniyalprashant9
Copy link
Contributor Author

@atherdon I am stuck at finding the absolute path :(
Module and test are in different folders so the relative path will not work when I send it as a parameter.

@atherdon
Copy link
Contributor

hmm, don't worry- it;s a common problem. why not compare it with other tests? did you tried this way?

@atherdon atherdon self-requested a review June 18, 2019 20:19
@atherdon
Copy link
Contributor

i actually think that this is a perfect way to show of how important tests and documentation is. because if you - as creator of this method - cannot deal with it - then something is not ok, right?

@atherdon
Copy link
Contributor

check this and tell me what i should do - try to help you or wait while you will figure it out on your own.

check #545

what i will do if i was you - redo that method few times. usually it helps to understand the problem better

@atherdon
Copy link
Contributor

it must work with ./sample or ../tests/sample it's definately something not ok with code

@uniyalprashant9
Copy link
Contributor Author

Screenshot (2)

It's working on my laptop.

@atherdon
Copy link
Contributor

hmm, then google why it can be broken - because travis ci always keeping our back save. this is why we need it - in order to test for different cases and set ups

@atherdon
Copy link
Contributor

still will advice to redo that method.

@uniyalprashant9
Copy link
Contributor Author

yes I am checking that.

@atherdon
Copy link
Contributor

ok, can you sync your fork with our master - because i see some conflicts?
https://github.com/ChickenKyiv/awesome-git-article#keeping-a-fork-up-to-date

@uniyalprashant9
Copy link
Contributor Author

something is messed up I will pull request 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.

2 participants