Mozmill fails oddly when things are not ordinary files or directories

RESOLVED FIXED

Status

Testing Graveyard
Mozmill
RESOLVED FIXED
7 years ago
2 years ago

People

(Reporter: Jeff Hammel, Assigned: Jeff Hammel)

Tracking

Details

(Whiteboard: [mozmill-2.0+])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

7 years ago
(mozmill)│mozmill -t <(echo 'hello world')
INFO Passed: 0
INFO Failed: 0
INFO Skipped: 0
Traceback (most recent call last):
  File
  "/home/jhammel/mozmill/src/mozmill/mozmill/mozmill/__init__.py",
  line 586, in run
    self.run_tests(MozMill, normal_tests, runner, results)
  File
  "/home/jhammel/mozmill/src/mozmill/mozmill/mozmill/__init__.py",
  line 556, in run_tests
    mozmill.run(tests)
  File
  "/home/jhammel/mozmill/src/mozmill/mozmill/mozmill/__init__.py",
  line 262, in run
    self.run_tests(tests)
  File
  "/home/jhammel/mozmill/src/mozmill/mozmill/mozmill/__init__.py",
  line 232, in run_tests
    tests += self.collect_tests(path)
  File
  "/home/jhammel/mozmill/src/mozmill/mozmill/mozmill/__init__.py",
  line 248, in collect_tests
    for filename in sorted(os.listdir(path)):
OSError: [Errno 20] Not a directory: '/dev/fd/63'

This is neither a file nor a directory in python's mind:

import os
import sys
f = sys.argv[1]
print os.path.isfile(f)
print os.path.isdir(f)

│python tmp.py <(echo 'hello world')
False
False

It's highly likely (albeit unknown to me) whether we can run these
things or not:

diff --git a/mozmill/mozmill/__init__.py b/mozmill/mozmill/__init__.py
index 818c9b4..8342401 100644
--- a/mozmill/mozmill/__init__.py
+++ b/mozmill/mozmill/__init__.py
@@ -241,7 +241,7 @@ class MozMill(object):
     def collect_tests(self, path):
         """find all tests for a given path"""
         
-        if os.path.isfile(path):
+        if not os.path.isdir(path):
           return [path]
 
         files = []

Running again:

(mozmill)│mozmill -t <(echo 'hello world')
INFO Passed: 0
INFO Failed: 0
INFO Skipped: 0

Not helpful :(

Maybe if its not `os.path.isfile` we should assert its a directory?
Other ideas?
(Assignee)

Updated

7 years ago
Whiteboard: [mozmill-2.0?]

Comment 1

7 years ago
(In reply to comment #0)

> Maybe if its not `os.path.isfile` we should assert its a directory?
> Other ideas?

Why not assert if what we're given is not a file and not a directory and throw an error?  (Command line validation style, so we don't even try to run the tests?)

Let's clean this up in 2.0
Whiteboard: [mozmill-2.0?] → [mozmill-2.0+]
(Assignee)

Comment 2

7 years ago
Yeah, that seems fine
(Assignee)

Updated

7 years ago
Assignee: nobody → jhammel
(Assignee)

Comment 3

7 years ago
Created attachment 521610 [details] [diff] [review]
Mozmill fails oddly when things are not ordinary files or directories
Attachment #521610 - Flags: review?(ahalberstadt)
Comment on attachment 521610 [details] [diff] [review]
Mozmill fails oddly when things are not ordinary files or directories

Sorry, have to r- as this breaks symbolic links.
Attachment #521610 - Flags: review?(ahalberstadt) → review-
(Assignee)

Comment 5

7 years ago
(In reply to comment #4)
> Comment on attachment 521610 [details] [diff] [review] [review]
> Mozmill fails oddly when things are not ordinary files or directories
> 
> Sorry, have to r- as this breaks symbolic links.

Links to files or links to directories or both?  Should be easy enough to account for, in theory
(Assignee)

Comment 6

7 years ago
It would be nice to have a test for this.
(Assignee)

Comment 7

7 years ago
Created attachment 535671 [details] [diff] [review]
this should respect symbolic links. does this solve the problem?
Attachment #521610 - Attachment is obsolete: true
Attachment #535671 - Flags: review?(ctalbert)
(Assignee)

Updated

7 years ago
Attachment #535671 - Flags: feedback?(halbersa)
Comment on attachment 535671 [details] [diff] [review]
this should respect symbolic links. does this solve the problem?

wfm
Attachment #535671 - Flags: feedback?(halbersa) → feedback+

Updated

7 years ago
Attachment #535671 - Flags: review?(ctalbert) → review+
(Assignee)

Comment 9

7 years ago
pushed to master: https://github.com/mozautomation/mozmill/commit/4536279cbd36cc1fa0b6c5f81ed8d231dba1b734

If this causes any downstream problems, we should fix.  Should be identical for non-symbolic links, so I think it shouldn't cause any problems.  If it does, we can probably easily fix.
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Product: Testing → Testing Graveyard
You need to log in before you can comment on or make changes to this bug.