| Message ID | 20200712144419.21457-3-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, On 12/07/2020 15:44, Laurent Pinchart wrote: > Add a test to verify file creation with File::open(). The test is > expected to fail as the File::open() implementation is not correct. > I guess as the previous two patches are not yet integrated, you could equally just merge 4/4 into 1/4 before and then it wouldn't fail ... But given the patches were done at different times, I can understand why you've built this on top :-) Still, Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > test/file.cpp | 28 ++++++++++++++++++++++++++-- > 1 file changed, 26 insertions(+), 2 deletions(-) > > diff --git a/test/file.cpp b/test/file.cpp > index 7688a9dc224a..f458f355ccad 100644 > --- a/test/file.cpp > +++ b/test/file.cpp > @@ -31,6 +31,7 @@ protected: > return TestFail; > > close(fd); > + unlink(fileName_.c_str()); > > return TestPass; > } > @@ -191,14 +192,37 @@ protected: > > file.close(); > > + /* Test file creation. */ > + file.setFileName(fileName_); > + > + if (file.exists()) { > + cerr << "Temporary file already exists" << endl; > + return TestFail; > + } > + > + if (file.open(File::ReadOnly)) { > + cerr << "Read-only open succeeded on nonexistent file" << endl; > + return TestFail; > + } > + > + if (!file.open(File::WriteOnly)) { > + cerr << "Write-only open failed on nonexistent file" << endl; > + return TestFail; > + } > + > + if (!file.exists()) { > + cerr << "Write-only open failed to create file" << endl; > + return TestFail; > + } > + > + file.close(); > + > /* Test read and write. */ > std::array<uint8_t, 256> buffer = { 0 }; > > strncpy(reinterpret_cast<char *>(buffer.data()), "libcamera", > buffer.size()); > > - file.setFileName(fileName_); > - > if (file.read(buffer) >= 0) { > cerr << "Read succeeded on closed file" << endl; > return TestFail; >
Hi Laurent, Thanks for your work. On 2020-07-12 17:44:18 +0300, Laurent Pinchart wrote: > Add a test to verify file creation with File::open(). The test is > expected to fail as the File::open() implementation is not correct. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > test/file.cpp | 28 ++++++++++++++++++++++++++-- > 1 file changed, 26 insertions(+), 2 deletions(-) > > diff --git a/test/file.cpp b/test/file.cpp > index 7688a9dc224a..f458f355ccad 100644 > --- a/test/file.cpp > +++ b/test/file.cpp > @@ -31,6 +31,7 @@ protected: > return TestFail; > > close(fd); > + unlink(fileName_.c_str()); > > return TestPass; > } > @@ -191,14 +192,37 @@ protected: > > file.close(); > > + /* Test file creation. */ > + file.setFileName(fileName_); > + > + if (file.exists()) { > + cerr << "Temporary file already exists" << endl; > + return TestFail; > + } > + > + if (file.open(File::ReadOnly)) { > + cerr << "Read-only open succeeded on nonexistent file" << endl; > + return TestFail; > + } > + > + if (!file.open(File::WriteOnly)) { > + cerr << "Write-only open failed on nonexistent file" << endl; > + return TestFail; > + } > + > + if (!file.exists()) { > + cerr << "Write-only open failed to create file" << endl; > + return TestFail; > + } > + > + file.close(); > + > /* Test read and write. */ > std::array<uint8_t, 256> buffer = { 0 }; > > strncpy(reinterpret_cast<char *>(buffer.data()), "libcamera", > buffer.size()); > > - file.setFileName(fileName_); > - > if (file.read(buffer) >= 0) { > cerr << "Read succeeded on closed file" << endl; > return TestFail; > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Kieran, On Sun, Jul 12, 2020 at 08:46:16PM +0100, Kieran Bingham wrote: > Hi Laurent, > > On 12/07/2020 15:44, Laurent Pinchart wrote: > > Add a test to verify file creation with File::open(). The test is > > expected to fail as the File::open() implementation is not correct. > > I guess as the previous two patches are not yet integrated, you could > equally just merge 4/4 into 1/4 before and then it wouldn't fail ... > > But given the patches were done at different times, I can understand why > you've built this on top :-) It's also because merging the test that reproduces the issue first allows making sure that the test correctly triggers the problem. If you merge the fix first, there's a risk that both the fix and the test could be incorrect, and cancel each other resulting in the test passing. > Still, > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > test/file.cpp | 28 ++++++++++++++++++++++++++-- > > 1 file changed, 26 insertions(+), 2 deletions(-) > > > > diff --git a/test/file.cpp b/test/file.cpp > > index 7688a9dc224a..f458f355ccad 100644 > > --- a/test/file.cpp > > +++ b/test/file.cpp > > @@ -31,6 +31,7 @@ protected: > > return TestFail; > > > > close(fd); > > + unlink(fileName_.c_str()); > > > > return TestPass; > > } > > @@ -191,14 +192,37 @@ protected: > > > > file.close(); > > > > + /* Test file creation. */ > > + file.setFileName(fileName_); > > + > > + if (file.exists()) { > > + cerr << "Temporary file already exists" << endl; > > + return TestFail; > > + } > > + > > + if (file.open(File::ReadOnly)) { > > + cerr << "Read-only open succeeded on nonexistent file" << endl; > > + return TestFail; > > + } > > + > > + if (!file.open(File::WriteOnly)) { > > + cerr << "Write-only open failed on nonexistent file" << endl; > > + return TestFail; > > + } > > + > > + if (!file.exists()) { > > + cerr << "Write-only open failed to create file" << endl; > > + return TestFail; > > + } > > + > > + file.close(); > > + > > /* Test read and write. */ > > std::array<uint8_t, 256> buffer = { 0 }; > > > > strncpy(reinterpret_cast<char *>(buffer.data()), "libcamera", > > buffer.size()); > > > > - file.setFileName(fileName_); > > - > > if (file.read(buffer) >= 0) { > > cerr << "Read succeeded on closed file" << endl; > > return TestFail;
diff --git a/test/file.cpp b/test/file.cpp index 7688a9dc224a..f458f355ccad 100644 --- a/test/file.cpp +++ b/test/file.cpp @@ -31,6 +31,7 @@ protected: return TestFail; close(fd); + unlink(fileName_.c_str()); return TestPass; } @@ -191,14 +192,37 @@ protected: file.close(); + /* Test file creation. */ + file.setFileName(fileName_); + + if (file.exists()) { + cerr << "Temporary file already exists" << endl; + return TestFail; + } + + if (file.open(File::ReadOnly)) { + cerr << "Read-only open succeeded on nonexistent file" << endl; + return TestFail; + } + + if (!file.open(File::WriteOnly)) { + cerr << "Write-only open failed on nonexistent file" << endl; + return TestFail; + } + + if (!file.exists()) { + cerr << "Write-only open failed to create file" << endl; + return TestFail; + } + + file.close(); + /* Test read and write. */ std::array<uint8_t, 256> buffer = { 0 }; strncpy(reinterpret_cast<char *>(buffer.data()), "libcamera", buffer.size()); - file.setFileName(fileName_); - if (file.read(buffer) >= 0) { cerr << "Read succeeded on closed file" << endl; return TestFail;
Add a test to verify file creation with File::open(). The test is expected to fail as the File::open() implementation is not correct. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- test/file.cpp | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-)