mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From James Peach <jpe...@apache.org>
Subject Re: Review Request 44947: Add tests for XFS project quota utilities.
Date Mon, 04 Apr 2016 17:27:04 GMT


> On April 1, 2016, 7:27 a.m., Jiang Yan Xu wrote:
> > src/tests/containerizer/xfs_quota_tests.cpp, lines 292-294
> > <https://reviews.apache.org/r/44947/diff/11/?file=1320117#file1320117line292>
> >
> >     Instead of doing cleanups here, we can register the projectIds with a member
variable:
> >     
> >     ```
> >     vector<prid_t> projectIds;
> >     ```
> >     
> >     This way in the `tearDown()` method we can go through all the them and call
`clearProjectQuota()` on each.

I don't think we need this anymore since we are constructing a fresh filesystem each time.


> On April 1, 2016, 7:27 a.m., Jiang Yan Xu wrote:
> > src/tests/containerizer/xfs_quota_tests.cpp, lines 273-280
> > <https://reviews.apache.org/r/44947/diff/11/?file=1320117#file1320117line273>
> >
> >     So for this we can also create
> >     
> >     ```
> >     directory
> >      |
> >      |- link
> >     ```
> >     
> >     Then:
> >     ```
> >     getProjectId(link);
> >     setProjectId(directory);
> >     getProjectId(link);
> >     clearProjectId(directory);
> >     getProjectId(link);
> >     ```
> >     
> >     Even if the reason for `EXPECT_ERROR(getProjectId(path));` to pass is due to
FTS_PHYSICAL (so we didn't attempt to set it on the link instead of we cannot set it on the
link), that's the situation we are ever going to run into in real scnearios anyways so it's
OK.

These tests were restructured due to feedback in other reviews.


> On April 1, 2016, 7:27 a.m., Jiang Yan Xu wrote:
> > src/tests/containerizer/xfs_quota_tests.cpp, line 215
> > <https://reviews.apache.org/r/44947/diff/11/?file=1320117#file1320117line215>
> >
> >     This test can be extended to include files:
> >     
> >     ```
> >     directory
> >       |
> >       |- file
> >     ```
> >     
> >     You can call `set|clearProjectId()` on the directory and verify `getProjectId()`
on the file inside (and the directory).

These tests were restructured due to feedback in other reviews.


> On April 1, 2016, 7:27 a.m., Jiang Yan Xu wrote:
> > src/tests/cluster.cpp, lines 434-436
> > <https://reviews.apache.org/r/44947/diff/11/?file=1320116#file1320116line434>
> >
> >     This can be tackled in another review because a) the tests here don't use cluster.cpp
and b) I see other issuse here as well (see below).
> 
> James Peach wrote:
>     The isolator tests use ``cluster.cpp``. I hit this when I had a bug in those tests
that passed invalid flags arguments to the Slave. Let me know whether you want me to move
or just drop this diff. However, it is a fairly annoying problem to debug.

Moved this change to a separate review.


- James


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44947/#review126334
-----------------------------------------------------------


On April 1, 2016, midnight, James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44947/
> -----------------------------------------------------------
> 
> (Updated April 1, 2016, midnight)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4828
>     https://issues.apache.org/jira/browse/MESOS-4828
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add tests for XFS project quota utilities.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
>   src/tests/cluster.cpp 2da0bd7612d571277e76d0a95ad8e776434af323 
>   src/tests/containerizer/xfs_quota_tests.cpp PRE-CREATION 
>   src/tests/environment.cpp 90dbe9488bda6af26143934e196aab0d69dccec3 
> 
> Diff: https://reviews.apache.org/r/44947/diff/
> 
> 
> Testing
> -------
> 
> Make check. Manual testing. These tests.
> 
> 
> Thanks,
> 
> James Peach
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message