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

added gitlab testing infrastructure to the test #3

Merged
merged 4 commits into from
Jul 27, 2019

Conversation

kosinovsky
Copy link
Contributor

added gitlab testing infrastructure to the test

@kosinovsky kosinovsky requested review from adammoody and CamStan June 24, 2019 17:59
@adammoody
Copy link
Contributor

Thanks, @kosinovsky . This was another test I wrote initially to do interactive debugging with TotalView. It's good for that, but let's improve it in a few ways for our automated testing. Would you please update this to do the following:

  1. after the flush, delete the source files
  2. during the fetch, check that each process gets back the set of filenames it expects
  3. write something unique to the file for each process before the flush, and then after the fetch read the files in and verify that the contents are exactly as the process expects

Thanks!

@kosinovsky
Copy link
Contributor Author

kosinovsky commented Jun 26, 2019 via email

@adammoody
Copy link
Contributor

In addition to the test to check filenames and file contents on a normal flush/fetch mentioned above, let's add a couple more tests.

Let's build a test in which we execute the async functions of filo.

Let's build a test in which we execute a flush, delete one of the destination files for one of the ranks, then execute a fetch. We should see that all processes detect that one process failed to read one of its files. Because there is an allreduce across all procs, the return code in Filo_Fetch should be an error on all process, even on those processes whose files were not deleted.

@kosinovsky
Copy link
Contributor Author

kosinovsky commented Jul 17, 2019 via email

@adammoody
Copy link
Contributor

@kosinovsky , yes "remove" would be fine as well.

@kosinovsky
Copy link
Contributor Author

kosinovsky commented Jul 17, 2019 via email

@adammoody
Copy link
Contributor

Oh, I see. I forgot that we were already deleting the source files. Yes, so step #1 of 3 is already done then.

@kosinovsky
Copy link
Contributor Author

kosinovsky commented Jul 17, 2019 via email

@kosinovsky
Copy link
Contributor Author

kosinovsky commented Jul 17, 2019 via email

test/test_filo.c Outdated Show resolved Hide resolved
test/test_filo_async.c Outdated Show resolved Hide resolved
@kosinovsky
Copy link
Contributor Author

kosinovsky commented Jul 25, 2019 via email

Copy link
Contributor

@adammoody adammoody left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @kosinovsky ! Looks good to me.

@CamStan
Copy link
Member

CamStan commented Jul 25, 2019

I'll take a look at this and add a travis file and then it should be good.

@kosinovsky
Copy link
Contributor Author

kosinovsky commented Jul 26, 2019 via email

@CamStan CamStan merged commit 4a70534 into master Jul 27, 2019
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