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

Now checking for suggestions file validity #47

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@lonesword
Copy link
Contributor

lonesword commented Jun 1, 2014

Finally got the tests to work. I have used a work around over the get_unique_filename() function in testcases.c. The tests used to fail because some filenames returned by get_unique_filename() did not exist, and I could not understand why there should be an infinite loop in the function (now commented out). The workaround is to create a new file by the generated name.

Also, the ./runtests still show 72%: Checks: 50, Failures: 0, Errors: 14
I am not getting any specific error messages (except the timeouts - 14 timeouts are the 14 reported errors?)

However, cloning your repo and running the tests produced the same results. So I hope nothing's seriously wrong here.

lonesword added some commits Jun 1, 2014

Added is_file_exists() to util.c that now checks for suggestions file…
… validity

Modified how get_unique_filename() in testcases.c works.
After implementing is_file_exists(), many tests failed because file names returned by get_unique_filename() did not exist.
To overcome this, get_unique_filename() now always create a dummy file with the generated unique name.
strbuf_addf(filename, "output/%d.test.vst",num);

if ( file_exist(strbuf_to_s(filename)) )
{

This comment has been minimized.

@lonesword

lonesword Jun 1, 2014

Author Contributor

calls get_unique_filename() recursively in case the generated file name already exists. Rather unlikely I guess

@@ -37,7 +40,16 @@ get_unique_filename()
strbuf_clear (filename);
}
}

This comment has been minimized.

@lonesword

lonesword Jun 1, 2014

Author Contributor

I couldn't understand this part.

  1. Generate file name
  2. If does not exist already, break
  3. If the file exists already, clear the buffer filename

And this is in an infinite loop.

@hargup

This comment has been minimized.

Copy link

hargup commented Jun 1, 2014

Got here from your blog. I noticed that you have sent a pull request for your master branch. Generally it is a good idea to keep the master branch clean. You should create a new branch from the master every time you work on something new and send a pull request for that branch. When something is changed in the main repository you pull into your master. That way your master always remains clean and updated. That is what we do at @sympy.

@lonesword

This comment has been minimized.

Copy link
Contributor Author

lonesword commented Jun 1, 2014

@hargup
Thanks a lot. In fact, I created a branch and merged it to my master and then submitted a PR. I didn't know that you could push from a branch to a remote's master. Thanks for the tip 👍

@navaneeth

This comment has been minimized.

Copy link
Member

navaneeth commented Jun 3, 2014

Hello Kevin,
After I fix the test timeout issues, the PR has conflicts. I'd suggest you to do the following after copying your changes to a text editor.

git reset --hard c50e4aaa227e9628e8742b61fe085be5bbb28ffc

Then sync your fork with my changes, Follow the instructions at https://help.github.com/articles/syncing-a-fork. This will update your fork with the latest changes went into libvarnam.

Now, create a new branch for your changes.

git checkout -b bug-4045

Apply your changes which you have kept in the text file back to the source files. Don't apply the change you did for infinite loop because that code is removed. It no longer does a infinite loop. Commit the changes and send a PR from the branch.

This is the proper way to develop features and submit PR. You can submit as many commits into the branch and the PR will get updated automatically. You can also amend to one commit and use --force while pushing. Something like,

git commit --amend
git push origin bug-4045 --force

This will update the PR with latest changes but keeping only one commit. I'd prefer this way because it keeps the commit history clean.

@navaneeth

This comment has been minimized.

Copy link
Member

navaneeth commented Jun 3, 2014

I will close this now. Please raise a new one with cleaned up commits after syncing the changes.

@navaneeth navaneeth closed this Jun 3, 2014

@lonesword

This comment has been minimized.

Copy link
Contributor Author

lonesword commented Jun 3, 2014

Thanks a lot for reviewing. I'll fetch ur changes and submit a new PR

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